Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RemoveUnusedBrs: Remove final block nops in all cases #4954

Merged
merged 1 commit into from
Aug 22, 2022
Merged

Conversation

kripken
Copy link
Member

@kripken kripken commented Aug 22, 2022

This fixes what looks like it might be a regression in #4943. It's not actually
an issue since it just affects wat files, but it did uncover an existing
inefficiency. The situation is this:

(block
  ..
  (br $somewhere)
  (nop)
)

Removing such a nop is always helpful, as the pass might see that that
br goes to where control flow is going anyhow, and the nop would
confuse it. We used to remove such nops only when the block had a name,
which is why wat testcases looks optimal, but we were actually doing the
less-efficient thing on real-world code. It was a minor inefficiency, though, as
the nop is quickly removed by later passes anyhow. Still, the fix is trivial (to
always remove such nops, regardless of a name on the block or not).

@kripken kripken requested a review from tlively August 22, 2022 21:02
@kripken kripken merged commit 0e0c2d9 into main Aug 22, 2022
@kripken kripken deleted the rubr-nop branch August 22, 2022 21:31
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.

2 participants