This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR69291, RTL if-conversion bug
- From: Richard Biener <rguenther at suse dot de>
- To: Bernd Schmidt <bschmidt at redhat dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Wed, 10 Feb 2016 14:35:55 +0100 (CET)
- Subject: Re: [PATCH] Fix PR69291, RTL if-conversion bug
- Authentication-results: sourceware.org; auth=none
- References: <alpine dot LSU dot 2 dot 11 dot 1602101400100 dot 31122 at t29 dot fhfr dot qr> <56BB3B64 dot 3040606 at redhat dot com> <alpine dot LSU dot 2 dot 11 dot 1602101430430 dot 31122 at t29 dot fhfr dot qr>
On Wed, 10 Feb 2016, Richard Biener wrote:
> 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);
Or less aggressive
Index: gcc/ifcvt.c
===================================================================
--- gcc/ifcvt.c (revision 233262)
+++ gcc/ifcvt.c (working copy)
@@ -1274,7 +1274,8 @@ 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))
+ || ! reg_mentioned_p (if_info->x, XEXP (a, 0)))
&& if_info->branch_cost >= 2)
{
common = XEXP (a, 0);