JIT: Make GS cookie phase run on LIR and move it after async phase #122973
Conversation
… transformation Functions with GS cookie checks may furthermore shadow parameters to make sure that the GS cookie ends up above all parameters on the stack. This is done by creating a copy of parameters used like a pointer and redirection all uses to act on the shadow. IR is inserted in the beginning of the function to copy the parameters to their shadows. For async this introduces a problem when it comes to implicit byrefs. The shadowing ceremony introduces a pointer to a local (the storage area in the caller) and that pointer remains live across suspension points, resulting in illegal IR. Ideally we would move the shadowing pass to run after the async transformation, but this requires rewriting the analysis from HIR to LIR. This change instead fixes the issue by keeping the analysis where it is, but by delaying the rewrite of the IR until after the async transformation (for async functions only).
github-actions
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
This comment was marked as outdated.
This comment was marked as outdated.
jakobbotsch
changed the title
|
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
cc @dotnet/jit-contrib PTAL @AndyAyersMS jitstress failure is #122481. The main change here is rewriting the analysis to work on LIR. Previously we started the tree walk at the root of each statement and kept track of the current context -- whether we are under an indirection or under a store -- and used it to either flag dependent locals as pointers or unify locals with destinations being stored to. There were a couple of sources of regressions after the move:
Diffs. Some minor improvements. |
AndyAyersMS
left a comment
AndyAyersMS
left a comment
There was a problem hiding this comment.
Looks good, just a few comments about comments.
4 participants
Functions with GS cookie checks may furthermore shadow parameters to make sure that the GS cookie ends up above all parameters on the stack. This is done by creating a copy of parameters used like a pointer and redirecting all uses to act on the shadow. IR is inserted in the beginning of the function to copy the parameters to their shadows.
For async this introduces a problem when it comes to implicit byrefs. The shadowing ceremony introduces a pointer to a local (the storage area in the caller) and that pointer remains live across suspension points, resulting in illegal IR.
This PR moves the GS phase so that it runs after the async transformation. To do so rewrite the analysis to run on LIR and the IR insertion/rewriting to be compatible with LIR.
Fix #122954