Remove GT_ARGPLACE nodes #68140
Remove GT_ARGPLACE nodes #68140
Conversation
These do not serve much purpose today -- instead just use null and add a helper function to iterate non-null early args, which is somewhat common. In addition to saving some TP and memory, teaching the backend about null early nodes will also be beneficial because I am planning to change rationalization to null out non-values in the early arg list so that all nodes have only values as their operands in LIR. PIN (libraries.pmi): Before: 316460895992 After: 315456532599 (-0.3%) Memory stats (libraries.pmi) Before: 25961399533 bytes After: 25770612141 bytes (-0.7%)
ghost
assigned 
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT Issue DetailsThese do not serve much purpose today -- instead just use null and add a In addition to saving some TP and memory, teaching the backend about PIN (libraries.pmi): Memory stats (libraries.pmi)
|
|
/azp run runtime-coreclr superpmi-replay, runtime-coreclr superpmi-asmdiffs |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
cc @dotnet/jit-contrib, any thoughts and does anyone want to take this one? |
| @@ -8352,33 +8348,30 @@ void Compiler::fgMorphRecursiveFastTailCallIntoLoop(BasicBlock* block, GenTreeCa | |||
| // Process early args. They may contain both setup statements for late args and actual args. | |||
| // Early args don't include 'this' arg. We need to account for that so that the call to gtArgEntryByArgNum | |||
| // below has the correct second argument. | |||
| for (CallArg& arg : recursiveTailCall->gtArgs.Args()) | |||
| for (CallArg& arg : recursiveTailCall->gtArgs.EarlyArgs()) | |||
There was a problem hiding this comment.
Is the comment above stale?
| @@ -3887,12 +3887,6 @@ GenTree* Compiler::fgSetTreeSeq(GenTree* tree, bool isLIR) | |||
| if (m_isLIR) | |||
| { | |||
| node->ClearReverseOp(); | |||
|
|
|||
| // ARGPLACE nodes are not threaded into the LIR sequence. | |||
There was a problem hiding this comment.
It's probably not worth re-running the CI for this, but the function header now has an outdated comment:
// isLIR - whether the sequencing is being done for LIR. If so,
// ARGPLACE nodes will not be threaded into the linear
// order, and the GTF_REVERSE_OPS flag will be cleared
There was a problem hiding this comment.
There was another instance of this and I also took the opportunity to change some uses of GetEarlyNode() to GetNode().
More obviously correct. Also add some asserts.
These do not serve much purpose today -- instead just use null and add a
helper function to iterate non-null early args, which is somewhat
common.
In addition to saving some TP and memory, teaching the backend about
null early nodes will also be beneficial because I am planning to change
rationalization to null out non-values in the early arg list so that all
nodes have only values as their operands in LIR.
No diffs and some nice throughput gains:
Memory stats (libraries.pmi)
https://www.diffchecker.com/0iigCXGB
Before: 25961399533 bytes
After: 25770612141 bytes (-0.7%)