diff mbox

Fix PR69291, RTL if-conversion bug

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

Commit Message

Richard Biener Feb. 10, 2016, 1:04 p.m. UTC
In this case if-conversion sees

 if ()
  (set (reg/v:SI 224 [ <retval> ])
        (plus:SI (plus:SI (reg/v:SI 160 [ mod_tlen ])
                (reg/v:SI 224 [ <retval> ]))
            (const_int 11 [0xb])
 else
  (set (reg/v:SI 224 [ <retval> ])
        (plus:SI (plus:SI (reg/v:SI 160 [ mod_tlen ])
                (reg/v:SI 224 [ <retval> ]))
            (const_int 10 [0xa])))

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.

The following patch fixes this.

Bootstrap and regtest running on x86_64-unknown-linux-gnu, I verified
this fixes my observed ruby miscompile on i586-linux.

Ok for trunk?

Thanks,
Richard.

2016-02-10  Richard Biener  <rguenther@suse.de>

	PR rtl-optimization/69291
	* ifcvt.c (noce_try_store_flag_constants): Properly handle
	common expressions.

Comments

Bernd Schmidt Feb. 10, 2016, 1:30 p.m. UTC | #1
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.


Bernd
diff mbox

Patch

Index: gcc/ifcvt.c
===================================================================
--- gcc/ifcvt.c	(revision 233262)
+++ gcc/ifcvt.c	(working copy)
@@ -1381,10 +1381,11 @@  noce_try_store_flag_constants (struct no
 
       /* 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;
 	}
 
       target = noce_emit_store_flag (if_info, if_info->x, reversep, normalize);