-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
JIT: Check for call interference when placing PUTARG nodes #103301
Conversation
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
Azure Pipelines successfully started running 2 pipeline(s). |
Btw, I wonder if it makes sense to introduce an assert that certain types of helper calls (backed by managed impl or with safe points) never appear in nogc regions |
Probably would, but note that the call arg setup isn't in a nogc region, so it wouldn't have caught this bug. |
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!
To ensure these maintain their relative ordering with their operand PUTARG nodes that may have been moved.
This adds a legalization pass for call argument
PUTARG
nodes after they have been created. We currently insert PUTARG_* nodes right after argument definitions, which is usually a good placement because morph ensured that this was ok.However, due to LIR transformations this placement might not be legal if there are any calls after the node. This PR moves the PUTARG_* nodes ahead of those interfering calls when they exist.
For example, for the case in #102919 we now end up with:
without this change the
PUTARG_STK
nodes end up before theBULKWRITEBARRIER
call.The algorithm works by first marking all
PUTARG
operand nodes of the call, and then walking backwards from the call in linear order until we have visited all marked nodes.If we see any call before visiting all marked nodes then we have found an interfering call, and the remaining
PUTARG_
nodes need to be moved after it.Fix #103300
Fix #102919
Fix #104042