On Wed, Jan 21, 2026 at 8:55 AM Andrew Pinski <[email protected]> wrote: > > 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.
OK, I guess that's reasonable. > I think moving this pass later should be delayed until GCC 17 because > of this reason. Agreed. It might be also a good enough reason not to move it. Unfortunately VRP is "last". DOM should eventually go away (once we finally get rid of its threadings). Richard. > 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 > > > >
