On Thu, Jan 15, 2026 at 1:01 AM Richard Biener
<[email protected]> wrote:
>
> On Thu, Jan 15, 2026 at 6:23 AM Andrew Pinski
> <[email protected]> wrote:
> >
> > This fixes the first part of SLSR incorrectly inserting undefined code
> > (overflow)
> > into the IR. The easiest way is to rewrite the statement after creating it
> > using rewrite_to_defined_unconditional.
> > This fixes the testcases from PR 121347 (and a few others) which all cause
> > an
> > infinite loops to appear.
> > I will be posting the fix for replace_rhs_if_not_dup later and at that
> > point I
> > will add a few testcases.
>
> OK.
>
> I have resisted on going this easy way due to fear of weakening further
> analysis. I do think that we eventually want to move SLSR a bit later,
> like at least after the VRP/threading blob of passes? Possibly right
> before the last forwprop?
So I looked what was further analysis at this point.
```
NEXT_PASS (pass_split_paths);
NEXT_PASS (pass_tracer);
NEXT_PASS (pass_fre, false /* may_iterate */);
/* After late FRE we rewrite no longer addressed locals into SSA
form if possible. */
NEXT_PASS (pass_thread_jumps, /*first=*/false);
NEXT_PASS (pass_dominator, false /* may_peel_loop_headers_p */);
NEXT_PASS (pass_strlen);
NEXT_PASS (pass_thread_jumps_full, /*first=*/false);
NEXT_PASS (pass_vrp, true /* final_p */);
/* Run CCP to compute alignment and nonzero bits. */
NEXT_PASS (pass_ccp, true /* nonzero_p */);
NEXT_PASS (pass_warn_restrict);
NEXT_PASS (pass_dse);
NEXT_PASS (pass_dce, true /* update_address_taken_p */, true /*
remove_unused_locals */);
/* After late DCE we rewrite no longer addressed locals into SSA
form if possible. */
NEXT_PASS (pass_forwprop, /*full_walk=*/false, /*last=*/true);
```
So after strength reduction only dom, fre and vrp could have a
difference (well thread for costing). and even currently dom and VRP
are the ones which causes the wrong code to show up. So moving it to
right before forwprop would make sense (forwprop does a copy prop
too). Plus with DCE right before you might end up with less issues in
the first place.
I will test a patch to move it later tonight.
Thanks,
Andrew
>
> > Bootstrapped and tested on x86_64-linux-gnu with no regressions.
> >
> > PR tree-optimization/121347
> > PR tree-optimization/106883
> > gcc/ChangeLog:
> >
> > * gimple-ssa-strength-reduction.cc (insert_initializers): Rewrite
> > newly inserted statements for undefinedness (overflow).
> >
> > Signed-off-by: Andrew Pinski <[email protected]>
> > ---
> > gcc/gimple-ssa-strength-reduction.cc | 31 +++++++++++++++++++++++-----
> > 1 file changed, 26 insertions(+), 5 deletions(-)
> >
> > diff --git a/gcc/gimple-ssa-strength-reduction.cc
> > b/gcc/gimple-ssa-strength-reduction.cc
> > index 70978b295d8..3bb924fa54c 100644
> > --- a/gcc/gimple-ssa-strength-reduction.cc
> > +++ b/gcc/gimple-ssa-strength-reduction.cc
> > @@ -57,6 +57,7 @@ along with GCC; see the file COPYING3. If not see
> > #include "tree-eh.h"
> > #include "builtins.h"
> > #include "tree-ssa-dce.h"
> > +#include "gimple-fold.h"
> >
> > /* Information about a strength reduction candidate. Each statement
> > in the candidate table represents an expression of one of the
> > @@ -3474,8 +3475,15 @@ insert_initializers (slsr_cand_t c)
> > gimple_set_location (cast_stmt, loc);
> > }
> >
> > - gsi_insert_before (&gsi, init_stmt, GSI_SAME_STMT);
> > gimple_set_location (init_stmt, loc);
> > + if (gimple_needing_rewrite_undefined (init_stmt))
> > + {
> > + gimple_seq seq;
> > + seq = rewrite_to_defined_unconditional (init_stmt);
> > + gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT);
> > + }
> > + else
> > + gsi_insert_before (&gsi, init_stmt, GSI_SAME_STMT);
> > }
> > else
> > {
> > @@ -3483,6 +3491,7 @@ insert_initializers (slsr_cand_t c)
> > gimple *basis_stmt = lookup_cand (c->basis)->cand_stmt;
> > location_t loc = gimple_location (basis_stmt);
> >
> > + gimple_set_location (init_stmt, gimple_location (basis_stmt));
> > if (!gsi_end_p (gsi) && stmt_ends_bb_p (gsi_stmt (gsi)))
> > {
> > if (cast_stmt)
> > @@ -3490,7 +3499,14 @@ insert_initializers (slsr_cand_t c)
> > gsi_insert_before (&gsi, cast_stmt, GSI_SAME_STMT);
> > gimple_set_location (cast_stmt, loc);
> > }
> > - gsi_insert_before (&gsi, init_stmt, GSI_SAME_STMT);
> > + if (gimple_needing_rewrite_undefined (init_stmt))
> > + {
> > + gimple_seq seq;
> > + seq = rewrite_to_defined_unconditional (init_stmt);
> > + gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT);
> > + }
> > + else
> > + gsi_insert_before (&gsi, init_stmt, GSI_SAME_STMT);
> > }
> > else
> > {
> > @@ -3499,10 +3515,15 @@ insert_initializers (slsr_cand_t c)
> > gsi_insert_after (&gsi, cast_stmt, GSI_NEW_STMT);
> > gimple_set_location (cast_stmt, loc);
> > }
> > - gsi_insert_after (&gsi, init_stmt, GSI_NEW_STMT);
> > + if (gimple_needing_rewrite_undefined (init_stmt))
> > + {
> > + gimple_seq seq;
> > + seq = rewrite_to_defined_unconditional (init_stmt);
> > + gsi_insert_seq_after (&gsi, seq, GSI_SAME_STMT);
> > + }
> > + else
> > + gsi_insert_after (&gsi, init_stmt, GSI_SAME_STMT);
> > }
> > -
> > - gimple_set_location (init_stmt, gimple_location (basis_stmt));
> > }
> >
> > if (dump_file && (dump_flags & TDF_DETAILS))
> > --
> > 2.43.0
> >
