On 10/13/26 5:26 AM, Jeff Law wrote:
On 10/12/26 8:51 PM, Vineet Gupta wrote:
hoist_code inserts a hoisted expression at the end of the target block,
which insert_insn_end_basic_block places before the block's final jump.
If that jump modifies one of the expression's operands, as an asm goto
does for its outputs and, with a "memory" clobber, for memory, the
hoisted expression reads the value from before the jump. Skip such
expressions.
Bootstrapped and regression tested on x86_64-linux-gnu and aarch64-linux-gnu
(Full disclosure: the testing was with combined fixes for 3 other PRs).
PR rtl-optimization/127847
gcc/ChangeLog:
* gcse.cc (hoist_code): Don't hoist an expression into a block
whose final jump insn modifies one of its operands.
gcc/testsuite/ChangeLog:
* gcc.dg/torture/asm-goto-hoist-1.c: New test.
* gcc.target/aarch64/asm-goto-hoist-2.c: New test.
* gcc.target/aarch64/asm-goto-hoist-3.c: New test.
* gcc.target/aarch64/asm-goto-hoist-4.c: New test.
* gcc.target/i386/asm-goto-hoist-2.c: New test.
* gcc.target/i386/asm-goto-hoist-3.c: New test.
* gcc.target/i386/asm-goto-hoist-4.c: New test.
Signed-off-by: Vineet Gupta <[email protected]>
Something seems off here. Yes, the problem is real (and we've fought a
closely related battle since the introduction of RTL GCSE/PRE back in
the late 90s.
In general we should be inserting on edges. The exception is abnormal
critical edges (which can't be split) and in those cases we try to fall
back to a correct, though perhaps partially redundant insertion point.
We also prune expressions from the hash tables that cause problems over
these edges.
One of those two mechanisms is more likely where we want to be fixing
this problem and I'd guess the latter is the more appropriate place to
fix. Look in gcse.cc for prune_expressions. If there's a hard reason
why that approach doesn't work, then something like what's done in this
patch starts to become more appropriate.
It seems prune_expressions is for pruning blocks with an incoming
abnormal edge. the right place for this handling. asm goto has ordinary
edges: bb 2 -> bb 3 is fallthru and bb 2 -> bb 4 (label) has no flags.
In fact looking in prune_expressions' comment,
" It would
be best to implement this check along the lines of
should_hoist_expr_to_dom where the target block is already known
and, hence, there's no need to conservatively prune expressions on
"intermediate" set-and-jump instructions."
which is what this patch does.
prune_expressions change will also clear the expression from anticipated
locally and transparent of jump's successor which can be more conservative ?
I have a proto-type which does work and is functionallyy equivalent -
longer in code and with the caveat above. I can share it if you want to ?
Thx,
-Vineet
