-
Notifications
You must be signed in to change notification settings - Fork 5.2k
JIT: Improve block IL offset range selection when splitting #121298
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
base: main
Are you sure you want to change the base?
Conversation
In debug codegen we expect basic blocks to have valid start IL offsets, potentially to an invalid IL end offset. When we split a block at a node, we find the end offset by looking backwards from that node for an `IL_OFFSET` node. If we find such a node, we use it as the split point IL offset. Otherwise we use `BAD_IL_OFFSET` and start the next block at the end offset (making it have a 0 byte range). This latter behavior is problematic if the bottom portion of the split has its own `IL_OFFSET` nodes. In that case we can end up with `IL_OFFSET` nodes pointing outside the region of the basic block. If we later split again, then we can end up creating illegal IL offset ranges for unoptimized code. In an example case we first ended up with the bottom block having an IL offset range of `[0aa..0aa)` but still containing an `IL_OFFSET` with an offset of `092`. When we later split the bottom block again we ended up with `[0aa..0aa), [092..0aa)` and hit an assert. To address the problem change the behavior in the following way: * If we find no `IL_OFFSET` node in the upper block, then look for one in the lower block, and use its value as the split point * If we find no `IL_OFFSET` in both blocks then consider the lower block empty and the upper block to have everything (same behavior as before). * One exception to above: if the end was already `BAD_IL_OFFSET` then we have to consider the upper block to be empty as we cannot represent the other case
|
PTAL @dotnet/jit-contrib. Failures are Android timeouts and known issues. |
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.
Pull Request Overview
This pull request improves the IL offset assignment logic when splitting basic blocks in LIR (Low-level Intermediate Representation) mode. The changes enhance how the compiler determines split point offsets by attempting to find valid IL_OFFSET nodes in both directions (backward and forward) before falling back to guessing.
Key changes:
- Adds forward search in the new block when backward search in the current block fails to find a valid IL_OFFSET
- Improves fallback logic to handle cases where no IL_OFFSET is found in either direction
- Changes split point offset assignment from using max/min calculations to direct assignment
Co-authored-by: Copilot <[email protected]>
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
In debug codegen we expect basic blocks to have valid start IL offsets, potentially to an invalid IL end offset.
When we split a block at a node, we find the end offset by looking backwards from that node for an
IL_OFFSETnode. If we find such a node, we use it as the split point IL offset. Otherwise we useBAD_IL_OFFSETand start the next block at the end offset (making it have a 0 byte range).This latter behavior is problematic if the bottom portion of the split has its own
IL_OFFSETnodes. In that case we can end up withIL_OFFSETnodes pointing outside the region of the basic block. If we later split again, then we can end up creating illegal IL offset ranges for unoptimized code. In an example case we first ended up with the bottom block having an IL offset range of[0aa..0aa)but still containing anIL_OFFSETwith an offset of092. When we later split the bottom block again we ended up with[0aa..0aa), [092..0aa)and hit an assert.To address the problem change the behavior in the following way:
IL_OFFSETnode in the upper block, then look for one in the lower block, and use its value as the split pointIL_OFFSETin both blocks then consider the lower block empty and the upper block to have everything (same behavior as before).BAD_IL_OFFSETthen we have to consider the upper block to be empty as we cannot represent the other caseThis fixes #119616. However for that specific case, which is inside async code, we will have the exact IL offset of the async call we are splitting at after #120303, so we will be able to do better. I'll make that change as part of that PR.