Add an assert concerning branch to CALLFINALLY blocks #55858
Conversation
ghost
added
the
area-CodeGen-coreclr
|
/azp run runtime-coreclr jitstress |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
This assert doesn't fire in SPMI, but does fire for the test case here: #55131 (comment). @AndyAyersMS @kunalspathak PTAL |
I'm not opposed to this, but we should acknowledge this is a choice, not a hard requirement ... unless you know of an actual unfixable correctness issue here. |
|
Looks like the assertion is hit during crossgen2 compilation of SPC.dll on windows x86. |
src/coreclr/jit/fgdiagnostic.cpp
Outdated
There was a problem hiding this comment.
From your comments, should this be under #ifdef FEATURE_EH_CALLFINALLY_THUNKS ?
True, this is a conservative statement about our existing EH flow model. Changing this to be more liberal requires more thinking about the implications. I can add a comment to that effect.
It should work for all cases, as the callee does the right thing in all cases. |
In the FEATURE_EH_CALLFINALLY_THUNKS case, BBJ_CALLFINALLY blocks live in the EH region enclosing the `try` block that needs to call the finally. However, we need all flow to the BBJ_CALLFINALLY to come from that try block; we don't want flow optimizations to otherwise branch directly to this BBJ_CALLFINALLY block. Add an assert to verify this is the case. The assert also covers the non-FEATURE_EH_CALLFINALLY_THUNKS case.
BruceForstall
force-pushed
the
AddBranchToCallFinallyAssert
branch
from
687b0a6 to
f8dff60
Compare
|
I verified that the new test in #55674 hits this assert, and succeeds without hitting this assert with the code fix in that PR is included in the JIT. |
|
/azp run runtime-coreclr jitstress |
|
Azure Pipelines successfully started running 1 pipeline(s). |
3 participants
In the FEATURE_EH_CALLFINALLY_THUNKS case, BBJ_CALLFINALLY blocks live
in the EH region enclosing the
tryblock that needs to call the finally.However, we need all flow to the CALLFINALLY to come from that try block;
we don't want flow optimizations to otherwise branch directly to this
CALLFINALLY block. Add an assert to verify this is the case.