JIT: Ensure EH clauses with same try region are reported contiguously to the VM #104531
Conversation
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
amanasifkhalid
changed the title
|
cc @janvorli @jkotas for visibility, @AndyAyersMS PTAL. Thanks! This change has a bunch of zero-size diffs from changes in EH clause ordering -- looks like the JIT's internal ordering was breaking up clauses with the same try region with some frequency. TP cost is pretty minimal. Are there any outerloop pipelines worth running? From my block layout work, I don't recall |
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| } | ||
|
|
||
| return leftTryIndex < rightTryIndex; | ||
| }); |
There was a problem hiding this comment.
We don't use any of the CORINFO_EH_CLAUSE data when sorting, so seems like it would be more efficient to just sort a table of EHblkDsc* and then report each clause like we did before?
There was a problem hiding this comment.
I initially tried doing this, and then iterating over the sorted table to build the clauses in the above loop, but changing the order in which we call Compiler::ehCodeOffset hit JIT asserts (in particular, assert(cookie != nullptr) in Compiler::ehEmitCookie) -- I think emitter::emitCodeOffset has some hard dependency on us calling ehCodeOffset in the order of the JIT's EH table. I can look into cleaning this up in a follow-up if you'd like, though the TP cost of the current approach doesn't seem too bad.
There was a problem hiding this comment.
Thats fine. EH is relatively rare and lots of EH clauses even more so.
I don't see why there'd be such an order dependence in the emitter, so it is worth digging into more deeply.
src/coreclr/jit/codegencommon.cpp
Outdated
| // We have two clauses mapped to the same try region. | ||
| // Make sure we report the clause with the smaller index first. | ||
| const auto leftIndex = left.HBtab - this->compiler->compHndBBtab; | ||
| const auto rightIndex = right.HBtab - this->compiler->compHndBBtab; |
There was a problem hiding this comment.
I don't think we have used auto for anything but function types. Use ptrdiff_t?
|
SPMI replay failure is #104570. |
The
CORINFO_EH_CLAUSE_SAMETRYflag indicates to the VM that a given EH clause maps to the same try region as the previous EH clause. However, the JIT's internal invariants may result in an EH table ordering such that clauses mapped to the same try region aren't contiguous, thus breaking the functionality of this flag. To address this without changing the JIT's EH invariants, ensure the JIT reports clauses with the same try region contiguously to the VM, and setsCORINFO_EH_CLAUSE_SAMETRYaccordingly.Fixes #101772.