On Mon, Sep 7, 2026 at 8:18 PM Andrea Pinski <[email protected]> wrote: > > On Mon, Sep 7, 2026 at 12:30 PM Dominic P <[email protected]> wrote: > > > > Follow-up to r17-3550, extending movement_possibility_1 to variable shift > > counts as suggested in > > https://gcc.gnu.org/pipermail/gcc-patches/2026-August/728576.html > > > > Querying ranger turns out not to be safe here. A contextual range need not > > hold at the location the statement moves to, and a global range can be > > derived from a __builtin_unreachable guard that has since been removed after > > bounding only the name's uses (tree-vrp.cc, remove_unreachable): an earlier > > version of this patch used the global range and hoisted a shift above such a > > guard, miscompiling function k in the new test. Instead accept a count > > whose defining statement is a BIT_AND_EXPR with a constant mask below the > > precision of the shifted type. This covers s &= 7 directly, and s % 8 where > > earlier passes rewrite it into that form. > > What about using the range on the incoming edge into the loop header? > That is using range_on_edge with loop_preheader_edge? > That should give you the range at the point. Maybe falling back to > this code as a fallout.
Actually when rewriting the shift that was hoisted, introduce a BIT_AND_EXPR instead? This way you can hoist the shift no matter what and you will have defined code still? if the value of the shift is not used then there will be no issues; if the value is used it will be the same as before. This might require more changes than your current check but I think it might be more beneficial and you can hoist the one for k still. > > Thanks, > Andrea > > > > > Bootstrapped on x86_64-pc-linux-gnu; regression tested there with > > gcc.dg/dg.exp and gcc.dg/tree-ssa/tree-ssa.exp, with no unexpected results. > > > > Assisted-by: Claude Opus 5 (Anthropic) > > > > gcc/ChangeLog: > > > > * tree-ssa-loop-im.cc (movement_possibility_1): Allow a shift > > or rotate by an SSA name whose defining statement masks the > > count below the precision of the shifted type. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.dg/tree-ssa/loop-im-shift-2.c: New test. > > > > Signed-off-by: Dominic P <[email protected]> > > --- > > .../gcc.dg/tree-ssa/loop-im-shift-2.c | 47 +++++++++++++++++++ > > gcc/tree-ssa-loop-im.cc | 35 ++++++++++---- > > 2 files changed, 73 insertions(+), 9 deletions(-) > > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/loop-im-shift-2.c > > > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/loop-im-shift-2.c > > b/gcc/testsuite/gcc.dg/tree-ssa/loop-im-shift-2.c > > new file mode 100644 > > index 00000000000..6555ef92ef4 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/tree-ssa/loop-im-shift-2.c > > @@ -0,0 +1,47 @@ > > +/* { dg-do compile } */ > > +/* { dg-options "-O2 -fdump-tree-lim2-details" } */ > > + > > +/* The count is masked into range, so the whole chain may be hoisted. */ > > + > > +void f (int *p, int n, int a, unsigned int s, int c, int *q) > > +{ > > + s &= 7; > > + for (int i = 0; i < n; i++) > > + if (p[i] > c) > > + q[i] = ((a << s) * 5) / 7; > > +} > > + > > +/* An unconstrained count keeps the chain in the loop. */ > > + > > +void g (int *p, int n, int a, unsigned int s, int c, int *q) > > +{ > > + for (int i = 0; i < n; i++) > > + if (p[i] > c) > > + q[i] = ((a << s) * 5) / 7; > > +} > > + > > +/* A mask that does not bound the count below the precision. */ > > + > > +void h (int *p, int n, int a, unsigned int s, int c, int *q) > > +{ > > + s &= 63; > > + for (int i = 0; i < n; i++) > > + if (p[i] > c) > > + q[i] = ((a << s) * 5) / 7; > > +} > > + > > +/* The global range of s comes from a guard the hoist would bypass. */ > > + > > +void k (int *p, int n, int a, unsigned int s, int c, int *q) > > +{ > > + for (int i = 0; i < n; i++) > > + if (p[i] > c) > > + { > > + if (s > 7) __builtin_unreachable (); > > + q[i] = ((a << s) * 5) / 7; > > + } > > +} > > + > > +/* Only f's chain moves: the shift, the multiply and the divide. */ > > +/* { dg-final { scan-tree-dump-times "Moving statement" 3 "lim2" } } */ > > +/* { dg-final { scan-tree-dump-times "Moving statement \[^\\r\\n\]*<<" 1 > > "lim2" } } */ > > diff --git a/gcc/tree-ssa-loop-im.cc b/gcc/tree-ssa-loop-im.cc > > index 0dcf5982ec2..a77241c8253 100644 > > --- a/gcc/tree-ssa-loop-im.cc > > +++ b/gcc/tree-ssa-loop-im.cc > > @@ -419,15 +419,32 @@ movement_possibility_1 (gimple *stmt) > > /* For shifts and rotates and possibly out-of-bound shift operands > > we currently cannot rewrite them into something unconditionally > > well-defined. */ > > - if ((code == LSHIFT_EXPR > > - || code == RSHIFT_EXPR > > - || code == LROTATE_EXPR > > - || code == RROTATE_EXPR) > > - && (TREE_CODE (gimple_assign_rhs2 (stmt)) != INTEGER_CST > > - /* We cannot use ranges at 'stmt' here. */ > > - || wi::geu_p (wi::to_wide (gimple_assign_rhs2 (stmt)), > > - element_precision (type)))) > > - ret = MOVE_PRESERVE_EXECUTION; > > + if (code == LSHIFT_EXPR > > + || code == RSHIFT_EXPR > > + || code == LROTATE_EXPR > > + || code == RROTATE_EXPR) > > + { > > + tree count = gimple_assign_rhs2 (stmt); > > + bool in_range = false; > > + if (TREE_CODE (count) == INTEGER_CST) > > + in_range = wi::ltu_p (wi::to_wide (count), > > + element_precision (type)); > > + else if (TREE_CODE (count) == SSA_NAME) > > + { > > + /* We cannot use ranges at 'stmt' here, contextual or global; > > + only a bound implied by the definition of the count holds > > + where the statement is moved to. */ > > + gimple *def = SSA_NAME_DEF_STMT (count); > > + if (is_gimple_assign (def) > > + && gimple_assign_rhs_code (def) == BIT_AND_EXPR > > + && TREE_CODE (gimple_assign_rhs2 (def)) == INTEGER_CST > > + && wi::ltu_p (wi::to_wide (gimple_assign_rhs2 (def)), > > + element_precision (type))) > > + in_range = true; > > + } > > + if (!in_range) > > + ret = MOVE_PRESERVE_EXECUTION; > > + } > > } > > > > /* Non local loads in a transaction cannot be hoisted out. Well, > > -- > > 2.55.0 > >
