-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Explicitly represent BBJ_EHFINALLYRET successors #93377
Explicitly represent BBJ_EHFINALLYRET successors #93377
Conversation
Currently, BBJ_EHFINALLYRET blocks have no explicit successors in the IR. To implement successor iteration, a very expensive process is followed to (1) find the region of blocks where a BBJ_CALLFINALLY block calling the `finally` might be found, (2) search the region for such blocks, and (3) return as a successor all the BBJ_ALWAYS blocks in the corresponding BBJ_CALLFINALLY/BBJ_ALWAYS pair. Change the IR to explicitly represent and maintain this list of successors for BBJ_EHFINALLYRET blocks. The representation is a simple array of `BasicBlock*`, similar to how BBJ_SWITCH block targets are represented. Fixes dotnet#84278 Notes: 1. The BBJ_EHFINALLYRET successors are computed in `impFixPredLists()`. There are various dumpers that run before this, so we need to tolerate incomplete successor information in some places. 2. `ehGetCallFinallyBlockRange()` is still used by some code. I changed the semantics to return a `[first..last]` range inclusive of `last` instead of the previous `[beginning..end)` range exclusive of `end`. This makes it easier to use with our BasicBlock iterators.
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsCurrently, BBJ_EHFINALLYRET blocks have no explicit successors in the IR. To implement successor iteration, a very expensive process is followed to (1) find the region of blocks where a BBJ_CALLFINALLY block calling the Change the IR to explicitly represent and maintain this list of successors for BBJ_EHFINALLYRET blocks. The representation is a simple array of Fixes #84278 Notes:
|
No diffs, generally a slight TP improvement, in some cases significant. |
@AndyAyersMS PTAL |
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.
Looks good overall. Left a few suggestions for possibly simplifying the code.
@AndyAyersMS I addressed your suggestions. |
The superpmi-diffs failure is out of disk space. |
Looks like @AndyAyersMS had a PR with the same win-arm64 failures: #93371 |
Maybe the build machines picked up a new MSVC arm64 compiler that has a bug? It does look like the compiler changed recently. |
Currently, BBJ_EHFINALLYRET blocks have no explicit successors in the IR. To implement successor iteration, a very expensive process is followed to (1) find the region of blocks where a BBJ_CALLFINALLY block calling the
finally
might be found, (2) search the region for such blocks, and (3) return as a successor all the BBJ_ALWAYS blocks in the corresponding BBJ_CALLFINALLY/BBJ_ALWAYS pair.Change the IR to explicitly represent and maintain this list of successors for BBJ_EHFINALLYRET blocks. The representation is a simple array of
BasicBlock*
, similar to how BBJ_SWITCH block targets are represented.Fixes #84278
Notes:
impFixPredLists()
. There are various dumpers that run before this, so we need to tolerate incomplete successor information in some places.ehGetCallFinallyBlockRange()
is still used by some code. I changed the semantics to return a[first..last]
range inclusive oflast
instead of the previous[beginning..end)
range exclusive ofend
. This makes it easier to use with our BasicBlock iterators.