Hello,
Thanks for the quick review!
On Tue, Sep 11, 2012 at 5:03 PM, Richard Henderson wrote:
> On 09/10/2012 04:26 PM, Steven Bosscher wrote:
>> + rtx index = force_reg (index_mode, dispatch_index);
>
> You can't modify the result of force_reg. Use copy_to_{mode_,}reg instead.
Done.
>> + rtx tmp = expand_simple_binop (index_mode, MINUS,
>> + index, CONST1_RTX (index_mode),
>> + index, 0, OPTAB_DIRECT);
>> + gcc_assert (REG_P (tmp));
>> + if (tmp != index)
>> + emit_move_insn (index, tmp);
>
> This pattern is force_expand_binop.
Didn't know about this one :-)
> Of course, you don't really need to force index be the same all
> the way down the chain. You could just as well use
>
> index = expand_simple_binop (index_mode, MINUS, index, one,
> index, 0, OPTAB_DIRECT);
>
> and use any new pseudo in the next iteration.
Right, I've made the changes to do so.
> Otherwise this looks good.
I made the following changes:
$ interdiff sjlj_tablejump.diff.20120910 sjlj_tablejump.diff
diff -u stmt.c stmt.c
--- stmt.c (working copy)
+++ stmt.c (working copy)
@@ -2129,19 +2129,16 @@
This is more efficient than a dispatch table on most machines.
The last "index--" is redundant but the code is trivially dead
and will be cleaned up by later passes. */
- rtx index = force_reg (index_mode, dispatch_index);
+ rtx index = copy_to_mode_reg (index_mode, dispatch_index);
rtx zero = CONST0_RTX (index_mode);
for (int i = 0; i < ncases; i++)
{
tree elt = VEC_index (tree, dispatch_table, i);
rtx lab = label_rtx (CASE_LABEL (elt));
do_jump_if_equal (index_mode, index, zero, lab, 0);
- rtx tmp = expand_simple_binop (index_mode, MINUS,
- index, CONST1_RTX (index_mode),
- index, 0, OPTAB_DIRECT);
- gcc_assert (REG_P (tmp));
- if (tmp != index)
- emit_move_insn (index, tmp);
+ force_expand_binop (index_mode, code_to_optab (MINUS),
+ index, CONST1_RTX (index_mode),
+ index, 0, OPTAB_DIRECT);
}
}
else
and I'm re-testing the updated patch. OK for trunk if it passes?
Ciao!
Steven
