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: Richard Sandiford <rdsandiford at googlemail dot com>
- Cc: Bernd Schmidt <bschmidt at redhat dot com>, gcc-patches at gcc dot gnu dot org
- Date: Tue, 16 Feb 2016 09:35:53 +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> <alpine dot LSU dot 2 dot 11 dot 1602101434590 dot 31122 at t29 dot fhfr dot qr> <56BB3EAB dot 2090805 at redhat dot com> <alpine dot LSU dot 2 dot 11 dot 1602101449290 dot 31122 at t29 dot fhfr dot qr> <56BB409D dot 8050808 at redhat dot com> <alpine dot LSU dot 2 dot 11 dot 1602101502410 dot 31122 at t29 dot fhfr dot qr> <874md9vmnl dot fsf at googlemail dot com>
On Mon, 15 Feb 2016, Richard Sandiford wrote:
> Richard Biener <rguenther@suse.de> writes:
> > On Wed, 10 Feb 2016, Bernd Schmidt wrote:
> >
> >> On 02/10/2016 02:50 PM, Richard Biener wrote:
> >> > On Wed, 10 Feb 2016, Bernd Schmidt wrote:
> >> >
> >> > > On 02/10/2016 02:35 PM, Richard Biener wrote:
> >> > >
> >> > > > 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)))
> >> > >
> >> > > I guess that would also work. Could maybe use a brief comment.
> >> >
> >> > Ok. I'm testing that. I wonder if we need to use reg_overlap_mentioned_p
> >> > here (hard-reg pairs?) or if reg_mentioned_p is safe.
> >>
> >> Let's go with reg_overlap_mentioned_p. I kind of forgot about that once I
> >> thought of possible issues with emitting a move :-(
> >
> > Ok, the following is in testing now.
> >
> > Ok?
> >
> > Thanks,
> > Richard.
> >
> > 2016-02-10 Richard Biener <rguenther@suse.de>
> >
> > PR rtl-optimization/69291
> > * ifcvt.c (noce_try_store_flag_constants): Do not allow
> > subexpressions affected by changing the result.
> >
> > Index: gcc/ifcvt.c
> > ===================================================================
> > --- gcc/ifcvt.c (revision 233262)
> > +++ gcc/ifcvt.c (working copy)
> > @@ -1274,7 +1274,10 @@ 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))
> > + /* Allow expressions that are not using the result or plain
> > + registers where we handle overlap below. */
> > + && (REG_P (XEXP (a, 0))
> > + || ! reg_overlap_mentioned_p (if_info->x, XEXP (a, 0)))
> > && if_info->branch_cost >= 2)
>
> Sorry if this has already been covered, but shouldn't we be adding
> to the noce_operand_ok check rather than replacing it? I think we
> still want to check side_effects_p and may_trap_p.
Whoops, yes. Testing fix.
Richard.