diff mbox

Fix PR69291, RTL if-conversion bug

Message ID alpine.LSU.2.11.1602101430430.31122@t29.fhfr.qr
State New
Headers show

Commit Message

Richard Biener Feb. 10, 2016, 1:33 p.m. UTC 
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


Thanks,
Richard.

Comments

Bernd Schmidt Feb. 10, 2016, 1:35 p.m. UTC | #1 
On 02/10/2016 02:33 PM, Richard Biener wrote:
>
> 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)

I think I prefer this, but actually maybe OBJECT_P is safe enough.


Bernd
diff mbox

Patch

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);