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. 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 >
