Skip to content

EffectAnalyzer: Do not reorder mayNotReturn with traps or global writes - #9211

Merged
tlively merged 1 commit into
mainfrom
binaryen-no-reorder-loop
Oct 5, 2026
Merged

tlively merged 1 commit into
mainfrom
binaryen-no-reorder-loop

Conversation

@tlively

@tlively tlively commented Oct 5, 2026

Copy link
Copy Markdown
Member

Expressions that may not return (loops, atomic.wait, struct.wait) can fail to terminate, which would prevent a subsequent trap or global state write (including calls) from ever executing. Update EffectAnalyzer::orderedBefore to disallow reordering mayNotReturn with traps or global writes, set mayNotReturn on AtomicWait, and clear loopEffects.mayNotReturn in LoopInvariantCodeMotion for the whole loop summary.

Fixes #9182.

Expressions that may not return (loops, atomic.wait, struct.wait) can fail to terminate, which would prevent a subsequent trap or global state write (including calls) from ever executing. Update EffectAnalyzer::orderedBefore to disallow reordering mayNotReturn with traps or global writes, set mayNotReturn on AtomicWait, and clear loopEffects.mayNotReturn in LoopInvariantCodeMotion for the whole loop summary.

Fixes #9182.
@tlively
tlively requested a review from a team as a code owner October 5, 2026 17:09
@tlively
tlively requested review from aheejin and removed request for a team October 5, 2026 17:09
Comment thread src/ir/effects.h
// Cannot reorder code that may not return with traps or global state
// changes.
if ((mayNotReturn && (other.trap || other.writesGlobalState())) ||
(other.mayNotReturn && (trap || writesGlobalState()))) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why handle traps and not throws etc?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The throws case and other control flow cases are already handled here: https://github.com/WebAssembly/binaryen/pull/9211/changes#diff-963609105fd78eff33b9674f5f72d45b5b8dbb8d8e4c866085fdd8c994b51a84R341-R345. mayNotReturn is already considered a side effect, so it is not reordered with respect to control flow.

@tlively
tlively merged commit 36a0c5e into main Oct 5, 2026
16 checks passed
@tlively
tlively deleted the binaryen-no-reorder-loop branch October 5, 2026 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EffectAnalyzer: code that may not return can be reordered with a trap or a call

2 participants