| Message ID | alpine.LSU.2.11.1602101502410.31122@t29.fhfr.qr |
|---|---|
| State | New |
| Headers | show |
On 02/10/2016 03:03 PM, Richard Biener wrote: > > 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. Ok if it passed. Bernd
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. Thanks, Richard
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.
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) { common = XEXP (a, 0);