Repository navigation
Split assignment rationalization into a separate phase - #85180
Conversation
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsAnd start adding the support required for store nodes to replace assignments in HIR. This change is the proper beginning to the Initially, I was concerned about the TP cost of introducing a new phase, and considered more complex schemes to achieve instrumentality, however, this ended up not being a problem - with some judicious use of PIN, we should be about even or slightly up on the TP metrics even with this change. Needless to say, once the end goal here has been achieved, we expect significant cumulative gains as the IR will shrink and special-casing from a number of places removed.
|
e06d10f to
d43de73
Compare
a1ad6a5 to
49e85c9
Compare
|
The TP situation (on 64 bit, 32 bit sees @dotnet/jit-contrib |
49e85c9 to
d1649af
Compare
|
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
|
Azure Pipelines successfully started running 2 pipeline(s). |
And make "OperIsStore" means exactly these opers: GT_STORE_LCL_VAR GT_STORE_LCL_FLD GT_STOREIND GT_STORE_BLK Notable: GT_STORE_DYN_BLK is excluded as it does not follow conventions of all other stores (e. g. is VOID-typed) and acts more like opaque atomic operations instead. Also fix an old bug with gtNodeHasSideEffects.
d1649af to
8e14aaf
Compare
Ooops, did not notice this... I've removed the stress bit, since it looks like moving rationalization earlier (thus, testing morph) will be easier than originally anticipated. |
No worries, I'll rerun it just to be safe. |
|
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
|
Azure Pipelines successfully started running 2 pipeline(s). |
jakobbotsch
left a comment
There was a problem hiding this comment.
LGTM. Agree that it's fine to take the TP regressions for now.
|
One unique failure in libraries stress, though hard to see it being related: |
And start adding the support required for store nodes to replace assignments in HIR.
This change is the proper beginning to the
ASGremoval project. The plan as of currently is to slowly move the newly introduced phase up the phase list, adding support to shared code (aka morph) where necessary and rewriting the rest as needed. In other words, there will be no intermediate states where bothASGand store nodes can appear in IR at once - the work will be split across phases, but not node types.Initially, I was concerned about the TP cost of introducing a new phase, and considered more complex schemes to achieve incrementality, however, this ended up not being a problem - with some judicious use of PIN, we should be about even or slightly up on the TP metrics even with this change. Needless to say, once the end goal here has been achieved, we expect significant cumulative gains as the IR will shrink and special-casing from a number of places removed.