Skip to content

Conversation

kg
Copy link
Member

@kg kg commented Jun 28, 2023

Right now conditional branches look like:

block (
  ... compute condition ...
  i32.eqz
  br_if // take branch
)

and null checks look like

block (
  ... load pointer ...
  br_if // exit block
  ... bailout ...
)

Since wasm doesn't have branch hinting, it's hard to be sure how to properly hint an unlikely branch. I suspect that using different opcode layouts might produce better performance here, so this PR changes the generated code for both kinds of branches and moves more of the codegen into the Cfg class.

Local benchmarking results were inconclusive, but that's to be expected. The impact of this would likely appear in browser-bench and the perf autofiling if it appears anywhere. On the bright side it also makes null checks and branches much easier to spot in traces, so that's nice.

@ghost
Copy link

ghost commented Jun 28, 2023

Tagging subscribers to this area: @BrzVlad, @kotlarmilos
See info in area-owners.md if you want to be subscribed.

Issue Details

Right now conditional branches look like:

block (
  ... compute condition ...
  i32.eqz
  br_if // take branch
)

and null checks look like

block (
  ... load pointer ...
  br_if // exit block
  ... bailout ...
)

Since wasm doesn't have branch hinting, it's hard to be sure how to properly hint an unlikely branch. I suspect that using different opcode layouts might produce better performance here, so this PR changes the generated code for both kinds of branches and moves more of the codegen into the Cfg class.

Local benchmarking results were inconclusive, but that's to be expected. The impact of this would likely appear in browser-bench and the perf autofiling if it appears anywhere. On the bright side it also makes null checks and branches much easier to spot in traces, so that's nice.

Author: kg
Assignees: -
Labels:

area-Codegen-Interpreter-mono

Milestone: -

@kg kg requested review from lambdageek, BrzVlad and kotlarmilos June 30, 2023 23:04
@kg kg merged commit 53a10ea into dotnet:main Jul 10, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Aug 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants