On Wed, 10 Feb 2016, Bernd Schmidt wrote:

> On 02/10/2016 02:04 PM, Richard Biener wrote:
> > where noce_try_store_flag_constants identifies
> > 
> > (plus:SI (reg/v:SI 160 [ mod_tlen ])
> >           (reg/v:SI 224 [ <retval> ]))
> > 
> > as "common" and then tries to detect the case where setting the
> > result would clobber that value.  It doesn't seem to expect
> > anything else than regs that can be equal to the destination though
> > which is clearly an oversight.
> 
> >         /* If we have x := test ? x + 3 : x + 4 then move the original
> >      x out of the way while we store flags.  */
> > -      if (common && rtx_equal_p (common, if_info->x))
> > +      if (common && reg_mentioned_p (if_info->x, common))
> >     {
> > -     common = gen_reg_rtx (mode);
> > -     noce_emit_move_insn (common, if_info->x);
> > +     rtx tem = gen_reg_rtx (mode);
> > +     noce_emit_move_insn (tem, common);
> > +     common = tem;
> >     }
> 
> I'm not so sure noce_emit_move_insn will reliably handle an arbitrary
> expression. I think a more conservative fix would be to disable this transform
> if common is not a reg.

I also wondered about this but then noce_emit_move_insn is quite elaborate
(calling into expanders eventually).

But if you prefer I can instead test the following

Index: gcc/ifcvt.c
===================================================================
--- gcc/ifcvt.c (revision 233262)
+++ gcc/ifcvt.c (working copy)
@@ -1274,7 +1274,7 @@ noce_try_store_flag_constants (struct no
       && CONST_INT_P (XEXP (a, 1))
       && CONST_INT_P (XEXP (b, 1))
       && rtx_equal_p (XEXP (a, 0), XEXP (b, 0))
-      && noce_operand_ok (XEXP (a, 0))
+      && REG_P (XEXP (a, 0))
       && if_info->branch_cost >= 2)
     {
       common = XEXP (a, 0);

Thanks,
Richard.

Reply via email to