-
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
Stop tracking genReturnBB
after global morph
#97130
Stop tracking genReturnBB
after global morph
#97130
Conversation
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsThe After global morph, clear the pointer and don't maintain it; it is no This leads to diffs, as unreachable return blocks get deleted. This can
|
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.
LGTM, but seems like win-x86 is unhappy about the removal of (even unreachable) monitor exits in synchronized methods? Presumably in those cases we can just report the entire method (assuming the info is used to exit the lock on exceptional flow by the VM)?
26427c7
to
04f5bcb
Compare
win-x86 handling of synchronized methods is unique: they aren't transformed to try/finally, but instead are reported to the VM via the GC information and the VM handles unlocking. I'll look into this... |
/azp run runtime-coreclr outerloop, runtime-coreclr jitstress |
Azure Pipelines successfully started running 2 pipeline(s). |
The `genReturnBB` pointer is set if a merged return block is created during `fgAddInternal` (so, quite early during compilation). It needs to be maintained (and the unreferenced block it points to needs to be kept around) until global morph, which hooks up flow to this block. After global morph, clear the pointer and don't maintain it; it is no longer needed. Later code that was checking for it can instead check for `BBJ_RETURN` blocks or `GT_RETURN` nodes. Also, remove the marking of the `genReturnBB` block as `BBF_DONT_REMOVE`. This leads to diffs, as unreachable return blocks get deleted. This can happen, say, if all other exits are tail calls. If that happens and we're in a case of needing a profiler hook, then the profile exit hook is also not needed (it would be unreachable).
The JIT still needs to report an end-of-synchronized-range offset in the GC info for x86 synchronized methods, so just create a new label at the end of all blocks. The VM will automatically exit the synchronization on an exceptional method exit.
996b5b6
to
fca4c50
Compare
/azp run runtime-coreclr outerloop, runtime-coreclr jitstress |
Azure Pipelines successfully started running 2 pipeline(s). |
Pretty significant size improvements. |
* Stop tracking `genReturnBB` after global morph The `genReturnBB` pointer is set if a merged return block is created during `fgAddInternal` (so, quite early during compilation). It needs to be maintained (and the unreferenced block it points to needs to be kept around) until global morph, which hooks up flow to this block. After global morph, clear the pointer and don't maintain it; it is no longer needed. Later code that was checking for it can instead check for `BBJ_RETURN` blocks or `GT_RETURN` nodes. Also, remove the marking of the `genReturnBB` block as `BBF_DONT_REMOVE`. This leads to diffs, as unreachable return blocks get deleted. This can happen, say, if all other exits are tail calls. If that happens and we're in a case of needing a profiler hook, then the profile exit hook is also not needed (it would be unreachable). * Allow x86 synchronized methods to have unreachable return blocks The JIT still needs to report an end-of-synchronized-range offset in the GC info for x86 synchronized methods, so just create a new label at the end of all blocks. The VM will automatically exit the synchronization on an exceptional method exit. * Create final IG before clearing GC info
The
genReturnBB
pointer is set if a merged return block is createdduring
fgAddInternal
(so, quite early during compilation). It needsto be maintained (and the unreferenced block it points to needs to be
kept around) until global morph, which hooks up flow to this block.
After global morph, clear the pointer and don't maintain it; it is no
longer needed. Later code that was checking for it can instead check
for
BBJ_RETURN
blocks orGT_RETURN
nodes. Also, remove the markingof the
genReturnBB
block asBBF_DONT_REMOVE
.This leads to diffs, as unreachable return blocks get deleted. This can
happen, say, if all other exits are tail calls. If that happens and we're
in a case of needing a profiler hook, then the profile exit hook is also
not needed (it would be unreachable).