-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
JIT: Set bbFalseTarget
upon BBJ_COND
initialization/modification
#96265
Conversation
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsPart of #93020. Previously,
|
cc @dotnet/jit-contrib. No asmdiffs, and small TP diffs. SPMI failures are due to timeouts on win-x86. I noticed that we only use |
// TODO-NoFallThrough: also set bbFalseTarget | ||
bbKind = BBJ_COND; | ||
bbTrueTarget = trueTarget; | ||
bbFalseTarget = bbNext; |
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.
Should this function take a falseTarget
parameter that is used to set bbFalseTarget
? Then, callers would be required to set it. Maybe later on when we remove bbNext
as fall through?
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.
That makes sense to me. I'll open a follow-up PR with that refactor; it might help others avoid hitting asserts around bbFalseTarget
by forcing them to set it when initializing bbTrueTarget
.
Thank you for the review!
Part of #93020. Previously,
bbFalseTarget
was hard-coded to matchbbNext
inBasicBlock::SetNext
. We still requirebbFalseTarget
to point to the next block forBBJ_COND
blocks, but I've removed the logic for updatingbbFalseTarget
fromSetNext
, and placed calls toSetFalseTarget
whereverbbFalseTarget
needs to be updated because theBBJ_COND
block has been created or moved relative to its false successor. This helps set us up to start removing logic that enforces the block's false successor is the next block.