-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
make EndSwitch flush cases to the EndSwitch offset #6072
Conversation
@@ -163,7 +163,7 @@ SwitchIRBuilder::BeginSwitch() | |||
void | |||
SwitchIRBuilder::EndSwitch(uint32 offset, uint32 targetOffset) | |||
{ | |||
FlushCases(targetOffset); | |||
FlushCases(offset); | |||
AssertMsg(m_caseNodes->Count() == 0, "Not all switch case nodes built by end of switch"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is offset/targetOffset here ? (I don't know this code)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
offset
is the location of the EndSwitch
instruction. targetOffset
is the location that EndSwitch
branches to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. LGTM.
… offset Merge pull request #6072 from MikeHolman:endswitchbug This makes it so that if targetOffset is breaking out of a loop we won't have problems (could happen in wasm). Now cases will go to EndSwitch, which just does direct branch to targetOffset. In normal cases, that will be cleaned up by RetargetBrToBr peep, so codegen doesn't change. Fixes #6037
This makes it so that if targetOffset is breaking out of a loop we won't have problems (could happen in wasm).
Now cases will go to EndSwitch, which just does direct branch to targetOffset. In normal cases, that will be cleaned up by RetargetBrToBr peep, so codegen doesn't change.
Fixes #6037