> On 12 Jun 2026, at 02:31, Jeffrey Law <[email protected]> wrote: > > On 6/10/2026 1:10 PM, [email protected] wrote: >> From: Kyrylo Tkachov <[email protected]> >> >> factor_out_conditional_load currently only runs from the sink pass. Also >> call >> it from phiopt itself when the join block of a diamond load PHI <*P, *Q> lies >> outside any loop: there is nothing to loop-vectorise there, so commoning the >> two arm loads into a single P' = PHI <P, Q>; result = *P' is a pure win and >> is >> cheap to attempt. >> >> This is restricted to: >> * the late phiopt run (!early_p), since after inlining the region may yet >> become part of a loop where the commoned load could inhibit >> vectorisation; >> * SSA-name arm pointers (a new REQUIRE_SSA_ARGS argument to >> factor_out_conditional_load), so that phiprop does not undo it. >> >> This triggers a few 100s times in SPEC2026 with no harmful effects. The >> llvm benchmarks improves slightly on my aarch64 machine (1.6%) >> >> Bootstrapped and tested on aarch64-none-linux-gnu. >> Ok for trunk after the prerequisites? >> >> Thanks, >> Kyrill >> >> Signed-off-by: Kyrylo Tkachov <[email protected]> >> >> gcc/ >> PR tree-optimization/125557 >> * tree-ssa-phiopt.h (conditional_load_factorable_p) >> (factor_out_conditional_load): Add a REQUIRE_SSA_ARGS parameter >> defaulting to false. >> * tree-ssa-phiopt.cc (conditional_load_factorable_p): Honour >> REQUIRE_SSA_ARGS. >> (factor_out_conditional_load): Pass it through. >> (pass_phiopt::execute): Call factor_out_conditional_load for >> out-of-loop diamonds in the late run. >> >> gcc/testsuite/ >> >> PR tree-optimization/125557 >> * gcc.dg/tree-ssa/phiopt-factor-load-1.c: New test. >> --- >> >> diff --git a/gcc/tree-ssa-phiopt.h b/gcc/tree-ssa-phiopt.h >> index 7383095f5a5..8f9b9edcdad 100644 >> --- a/gcc/tree-ssa-phiopt.h >> +++ b/gcc/tree-ssa-phiopt.h >> @@ -25,11 +25,13 @@ extern int find_different_opnum (const gimple_match_op >> &arg0_op, >> /* True if factor_out_conditional_load would factor the load PHI at MERGE; >> lets >> the sink pass gate on commonability without mutating. */ >> extern bool conditional_load_factorable_p (edge e0, edge e1, basic_block >> merge, >> - gphi *phi); >> + gphi *phi, >> + bool require_ssa_args = false); > I tend to prefer fixing caller rather than defaulting arguments. If > there's only a few call sites just fix them. Similarly for > factor_out_conditional_load. > > OK when prereqs go in. Ideally with the defaulted arguments fixed too.
Thanks for the reviews from all of you. I’ve tried to incorporate all the comments and ended up merging the patches into one patch that runs during sink_code_in_bb that hopefully addresses all the comments. I’ve sent it separately. Kyrill > > jeff
