On Sun, May 4, 2025 at 11:51 PM Richard Biener <richard.guent...@gmail.com> wrote: > > On Mon, May 5, 2025 at 3:42 AM Andrew Pinski <quic_apin...@quicinc.com> wrote: > > > > While fixing up how rewrite_to_defined_overflow works, > > gcc.dg/Wrestrict-22.c started > > to fail. This is because `d p+ 2` would moved by LIM and then be rewritten > > not using > > pointer plus. The rewriting part is correct behavior. It only recently > > started to be > > moved out; due to r16-190-g6901d56fea2132. > > Which has the following comment: > > ``` > > When we run before PRE and PRE is active hoist all expressions > > since PRE would do so anyway and we can preserve range info > > but PRE cannot. > > ``` > > But LIM will not preserve range info when moving conditional executed > > statements out of the loop. So it would be better not to override the cost > > check > > for conditional executed statements in the first place. > > > > Bootstrapped and tested on x86_64-linux-gnu. > > > > gcc/ChangeLog: > > > > * tree-ssa-loop-im.cc (compute_invariantness): Don't ignore the > > cost for > > conditional executed statements. > > > > Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com> > > --- > > gcc/tree-ssa-loop-im.cc | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/gcc/tree-ssa-loop-im.cc b/gcc/tree-ssa-loop-im.cc > > index a3ca5af3e3e..444ee31242f 100644 > > --- a/gcc/tree-ssa-loop-im.cc > > +++ b/gcc/tree-ssa-loop-im.cc > > @@ -1244,8 +1244,13 @@ compute_invariantness (basic_block bb) > > if (lim_data->cost >= LIM_EXPENSIVE > > /* When we run before PRE and PRE is active hoist all expressions > > since PRE would do so anyway and we can preserve range info > > - but PRE cannot. */ > > - || (flag_tree_pre && !in_loop_pipeline)) > > + but PRE cannot. Except for conditional statements as those > > will never > > + preserve range info. */ > > + || (flag_tree_pre && !in_loop_pipeline > > + && ALWAYS_EXECUTED_IN (bb) > > + && (ALWAYS_EXECUTED_IN (bb) == lim_data->max_loop > > So that's not exactly the same condition as we use in the end - we should > be able to set profitability to the level the stmt is always executed in > instead by using set_level (stmt, gimple_bb (stmt)->loop_father, > ALWAYS_EXECUTED_IN (bb)), > of course only when lim_data->cost < LIM_EXPENSIVE, so I'd split up the > if into two cases, profitable cost-wise and the PRE exception.
Yes, that makes sense. Testing a new patch with the above fixed. Thanks, Andrew > > > + || flow_loop_nested_p (ALWAYS_EXECUTED_IN (bb), > > + lim_data->max_loop)))) > > set_profitable_level (stmt); > > } > > } > > -- > > 2.34.1 > >
