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
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? |
AndyAyersMS
left a comment
AndyAyersMS
left a comment
There was a problem hiding this comment.
Thanks. Another good cleanup.
| // 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?
| { | ||
| 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.
3 participants
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%)