-
Notifications
You must be signed in to change notification settings - Fork 5k
JIT: enhance SSA checker to check some PHI properties #85533
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
Conversation
* Ensure PhiArgs have the right local number. * Ensure PhiArgs have unique `gtPredBB` (for most blocks) Handler entries may have multiple PhiArgs with the same `gtPredBB`. This is by design, to model exceptional flow from the middle of the block to a handler. Jump threading relies on there being just a single PhiArg per pred. So exclude handler entry blocks from jump threading. Prep work for possibly changing SSA to produce one PhiArg per pred, instead of one PhiArg per ssa def.
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue Details
Handler entries may have multiple PhiArgs with the same Jump threading relies on there being just a single PhiArg per pred. So exclude handler entry blocks from jump threading. Prep work for possibly changing SSA to produce one PhiArg per pred, instead of one PhiArg per ssa def.
|
@jakobbotsch PTAL No diffs expected. |
src/coreclr/jit/fgdiagnostic.cpp
Outdated
for (Statement* const stmt : block->Statements()) | ||
{ | ||
if (!stmt->IsPhiDefnStmt()) | ||
{ | ||
break; | ||
} |
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.
We already have NonPhiStatements(); maybe we should have PhiStatements()? (Presumably somebody else would verify all the PHI statements appear first).
for (Statement* const stmt : block->Statements()) | |
{ | |
if (!stmt->IsPhiDefnStmt()) | |
{ | |
break; | |
} | |
for (Statement* const stmt : block->PhiStatements()) | |
{ |
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.
I should probably check that all the PhiDefs appear first, but then I wouldn't need the partial iterator.
Failure looks similar to #85082 but the pattern there is different.
|
gtPredBB
(for most blocks)Handler entries may have multiple PhiArgs with the same
gtPredBB
. This is by design, to model exceptional flow from the middle of the block to a handler.Jump threading relies on there being just a single PhiArg per pred. So exclude handler entry blocks from jump threading.
Prep work for possibly changing SSA to produce one PhiArg per pred, instead of one PhiArg per ssa def.