This is an archive of the discontinued LLVM Phabricator instance.

[SimplifyCFG] Improve the way hoisting skips over non-matching instructions
ClosedPublic

Authored by foad on Apr 27 2023, 11:12 AM.

Details

Summary

D129370 introduced the idea that hoisting could skip over non-matching
instructions and continue to look for matching (hoistable) instructions,
but certain types of mismatch still aborted the whole hoisting attempt.

Fix this by splitting out some of the instruction matching checks into a
helper function.

Also forbid hoisting allocas past stacksave/stackrestore, completing the
fix started in D133730, to avoid regressing tests.

Diff Detail

Event Timeline

foad created this revision.Apr 27 2023, 11:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 27 2023, 11:12 AM
foad requested review of this revision.Apr 27 2023, 11:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 27 2023, 11:12 AM
foad added a subscriber: aeubanks.
foad added inline comments.
llvm/test/Transforms/SimplifyCFG/hoist-common-skip.ll
394

This looks wrong since this alloca was hoisted past a stacksave. D133730 was supposed to prevent this, according to its description, but in fact it only seems to prevent the converse: hoisting a stacksave past an alloca. @aeubanks

efriedma added inline comments.Apr 27 2023, 11:37 AM
llvm/test/Transforms/SimplifyCFG/hoist-common-skip.ll
394

Forbidding hoisting allocas past side-effects seems appropriate, yes. Should be straightforward.

foad updated this revision to Diff 517688.Apr 27 2023, 1:06 PM

Forbid hoisting allocas past stacksave/stackrestore.

foad edited the summary of this revision. (Show Details)Apr 27 2023, 1:07 PM
This revision is now accepted and ready to land.Apr 27 2023, 1:09 PM