On Tue, Jan 20, 2026 at 3:27 PM Andrew Pinski <[email protected]> wrote: > > 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.
So I tested the patch to move it but it looks like SLSR depends on FRE/DOM to cleanup some extra insertions. Take gcc.dg/tree-ssa/slsr-7.c. SLSR will insert (or the one with the overflow fixed up): slsr_12 = s_2(D) * 2; Even though there is already: a1_3 = s_2(D) * 2; gcc.dg/tree-ssa/slsr-2[78].c have the same issue, SLSR depends on FRE/DOM to do the CSE afterwards. I think moving this pass later should be delayed until GCC 17 because of this reason. Thanks, Andrew > > 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 > > >
