Skip to content

Commit

Permalink
Remove GT_ARGPLACE nodes (#68140)
Browse files Browse the repository at this point in the history
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.

Throughput diff:
```
Collection                                  Base # instructions Diff # instructions PDIFF
aspnet.run.windows.x64.checked.mch           69,717,468,395      69,206,312,087     -0.73%
benchmarks.run.windows.x64.checked.mch       54,695,846,729      54,294,078,768     -0.73%
coreclr_tests.pmi.windows.x64.checked.mch   340,169,515,528     337,478,749,067     -0.79%
libraries.crossgen2.windows.x64.checked.mch 128,653,906,043     126,926,566,191     -1.34%
libraries.pmi.windows.x64.checked.mch       228,653,702,806     226,554,618,843     -0.92%
libraries_tests.pmi.windows.x64.checked.mch 531,053,530,645     525,233,144,101     -1.10%
```

Memory stats (libraries.pmi)
Before: 25961399533 bytes
After: 25770612141 bytes (-0.7%)
  • Loading branch information
jakobbotsch authored Apr 23, 2022
1 parent 10ac54d commit bb6954e
Show file tree
Hide file tree
Showing 34 changed files with 230 additions and 350 deletions.
8 changes: 3 additions & 5 deletions docs/design/coreclr/jit/jit-call-morphing.md
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,9 @@ For arguments that are marked as not needing a temp:
-----------------

1. If this is an argument that is passed in a register, then the existing
node is moved to the late argument list and a new `GT_ARGPLACE` (placeholder)
node replaces it in the early argument list.
2. Additionally, if `m_needPlace` is true (only for `FEATURE_FIXED_OUT_ARGS`)
then the existing node is moved to the late argument list and a new
`GT_ARGPLACE` (placeholder) node replaces it in the `early argument list.
node is moved to the late argument list.
2. Similarly, if `m_needPlace` is true (only for `FEATURE_FIXED_OUT_ARGS`)
then the existing node is moved to the late argument list.
3. Otherwise the argument is left in the early argument and it will be
evaluated directly into the outgoing arg area or pushed on the stack.

Expand Down
7 changes: 0 additions & 7 deletions src/coreclr/jit/codegenarmarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5478,13 +5478,6 @@ unsigned CodeGenInterface::InferStructOpSizeAlign(GenTree* op, unsigned* alignme
opSize = TARGET_POINTER_SIZE * 2;
alignment = TARGET_POINTER_SIZE;
}
else if (op->IsArgPlaceHolderNode())
{
CORINFO_CLASS_HANDLE clsHnd = op->AsArgPlace()->gtArgPlaceClsHnd;
assert(clsHnd != 0);
opSize = roundUp(compiler->info.compCompHnd->getClassSize(clsHnd), TARGET_POINTER_SIZE);
alignment = roundUp(compiler->info.compCompHnd->getClassAlignmentRequirement(clsHnd), TARGET_POINTER_SIZE);
}
else
{
assert(!"Unhandled gtOper");
Expand Down
7 changes: 0 additions & 7 deletions src/coreclr/jit/codegenlinear.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1328,13 +1328,6 @@ void CodeGen::genConsumeRegAndCopy(GenTree* node, regNumber needReg)
void CodeGen::genNumberOperandUse(GenTree* const operand, int& useNum) const
{
assert(operand != nullptr);

// Ignore argument placeholders.
if (operand->OperGet() == GT_ARGPLACE)
{
return;
}

assert(operand->gtUseNum == -1);

if (!operand->isContained() && !operand->IsCopyOrReload())
Expand Down
7 changes: 0 additions & 7 deletions src/coreclr/jit/codegenloongarch64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9343,13 +9343,6 @@ unsigned CodeGenInterface::InferStructOpSizeAlign(GenTree* op, unsigned* alignme
opSize = TARGET_POINTER_SIZE * 2;
alignment = TARGET_POINTER_SIZE;
}
else if (op->IsArgPlaceHolderNode())
{
CORINFO_CLASS_HANDLE clsHnd = op->AsArgPlace()->gtArgPlaceClsHnd;
assert(clsHnd != 0);
opSize = roundUp(compiler->info.compCompHnd->getClassSize(clsHnd), TARGET_POINTER_SIZE);
alignment = roundUp(compiler->info.compCompHnd->getClassAlignmentRequirement(clsHnd), TARGET_POINTER_SIZE);
}
else
{
assert(!"Unhandled gtOper");
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/codegenxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5377,7 +5377,7 @@ void CodeGen::genCall(GenTreeCall* call)
// The call will pop its arguments.
// for each putarg_stk:
target_ssize_t stackArgBytes = 0;
for (CallArg& arg : call->gtArgs.Args())
for (CallArg& arg : call->gtArgs.EarlyArgs())
{
GenTree* argNode = arg.GetEarlyNode();
if (argNode->OperIs(GT_PUTARG_STK) && ((argNode->gtFlags & GTF_LATE_ARG) == 0))
Expand Down
5 changes: 1 addition & 4 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -2605,8 +2605,6 @@ class Compiler

GenTree* gtNewNothingNode();

GenTree* gtNewArgPlaceHolderNode(var_types type, CORINFO_CLASS_HANDLE clsHnd);

GenTree* gtUnusedValNode(GenTree* expr);

GenTree* gtNewKeepAliveNode(GenTree* op);
Expand Down Expand Up @@ -10651,7 +10649,6 @@ class GenTreeVisitor
case GT_JMPTABLE:
case GT_CLS_VAR:
case GT_CLS_VAR_ADDR:
case GT_ARGPLACE:
case GT_PHYSREG:
case GT_EMITNOP:
case GT_PINVOKE_PROLOG:
Expand Down Expand Up @@ -10838,7 +10835,7 @@ class GenTreeVisitor
{
GenTreeCall* const call = node->AsCall();

for (CallArg& arg : call->gtArgs.Args())
for (CallArg& arg : call->gtArgs.EarlyArgs())
{
result = WalkTree(&arg.EarlyNodeRef(), call);
if (result == fgWalkResult::WALK_ABORT)
Expand Down
12 changes: 1 addition & 11 deletions src/coreclr/jit/compiler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1256,15 +1256,6 @@ inline void GenTree::gtBashToNOP()
gtFlags &= ~(GTF_ALL_EFFECT | GTF_REVERSE_OPS);
}

// return new arg placeholder node. Does not do anything but has a type associated
// with it so we can keep track of register arguments in lists associated w/ call nodes

inline GenTree* Compiler::gtNewArgPlaceHolderNode(var_types type, CORINFO_CLASS_HANDLE clsHnd)
{
GenTree* node = new (this, GT_ARGPLACE) GenTreeArgPlace(type, clsHnd);
return node;
}

/*****************************************************************************/

inline GenTree* Compiler::gtUnusedValNode(GenTree* expr)
Expand Down Expand Up @@ -4208,7 +4199,6 @@ void GenTree::VisitOperands(TVisitor visitor)
case GT_JMPTABLE:
case GT_CLS_VAR:
case GT_CLS_VAR_ADDR:
case GT_ARGPLACE:
case GT_PHYSREG:
case GT_EMITNOP:
case GT_PINVOKE_PROLOG:
Expand Down Expand Up @@ -4370,7 +4360,7 @@ void GenTree::VisitOperands(TVisitor visitor)
{
GenTreeCall* const call = this->AsCall();

for (CallArg& arg : call->gtArgs.Args())
for (CallArg& arg : call->gtArgs.EarlyArgs())
{
if (visitor(arg.GetEarlyNode()) == VisitResult::Abort)
{
Expand Down
6 changes: 5 additions & 1 deletion src/coreclr/jit/fgdiagnostic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3047,7 +3047,11 @@ void Compiler::fgDebugCheckFlags(GenTree* tree)
{
// TODO-Cleanup: this is a patch for a violation in our GT_ASG propagation.
// see https://github.com/dotnet/runtime/issues/13758
actualFlags |= arg.GetEarlyNode()->gtFlags & GTF_ASG;
if (arg.GetEarlyNode() != nullptr)
{
actualFlags |= arg.GetEarlyNode()->gtFlags & GTF_ASG;
}

if (arg.GetLateNode() != nullptr)
{
actualFlags |= arg.GetLateNode()->gtFlags & GTF_ASG;
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/fgprofile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1560,6 +1560,7 @@ class ClassProbeInserter
uint8_t* classProfile = m_schema[*m_currentSchemaIndex].Offset + m_profileMemory;
*m_currentSchemaIndex += 2; // There are 2 schema entries per class probe

assert(!call->gtArgs.AreArgsComplete());
CallArg* objUse = nullptr;
if (compiler->impIsCastHelperEligibleForClassProbe(call))
{
Expand Down
27 changes: 10 additions & 17 deletions src/coreclr/jit/flowgraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1057,7 +1057,8 @@ GenTree* Compiler::fgOptimizeDelegateConstructor(GenTreeCall* call,

assert(call->gtArgs.HasThisPointer());
assert(call->gtArgs.CountArgs() == 3);
GenTree* targetMethod = call->gtArgs.GetArgByIndex(2)->GetEarlyNode();
assert(!call->gtArgs.AreArgsComplete());
GenTree* targetMethod = call->gtArgs.GetArgByIndex(2)->GetNode();
noway_assert(targetMethod->TypeGet() == TYP_I_IMPL);
genTreeOps oper = targetMethod->OperGet();
CORINFO_METHOD_HANDLE targetMethodHnd = nullptr;
Expand All @@ -1071,7 +1072,7 @@ GenTree* Compiler::fgOptimizeDelegateConstructor(GenTreeCall* call,
else if (oper == GT_CALL && targetMethod->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_VIRTUAL_FUNC_PTR))
{
assert(targetMethod->AsCall()->gtArgs.CountArgs() == 3);
GenTree* handleNode = targetMethod->AsCall()->gtArgs.GetArgByIndex(2)->GetEarlyNode();
GenTree* handleNode = targetMethod->AsCall()->gtArgs.GetArgByIndex(2)->GetNode();

if (handleNode->OperGet() == GT_CNS_INT)
{
Expand Down Expand Up @@ -1110,7 +1111,7 @@ GenTree* Compiler::fgOptimizeDelegateConstructor(GenTreeCall* call,
GenTreeCall* runtimeLookupCall = qmarkNode->AsOp()->gtOp2->AsOp()->gtOp1->AsCall();

// This could be any of CORINFO_HELP_RUNTIMEHANDLE_(METHOD|CLASS)(_LOG?)
GenTree* tokenNode = runtimeLookupCall->gtArgs.GetArgByIndex(1)->GetEarlyNode();
GenTree* tokenNode = runtimeLookupCall->gtArgs.GetArgByIndex(1)->GetNode();
noway_assert(tokenNode->OperGet() == GT_CNS_INT);
targetMethodHnd = CORINFO_METHOD_HANDLE(tokenNode->AsIntCon()->gtCompileTimeHandle);
}
Expand Down Expand Up @@ -1142,8 +1143,8 @@ GenTree* Compiler::fgOptimizeDelegateConstructor(GenTreeCall* call,
{
JITDUMP("optimized\n");

GenTree* thisPointer = call->gtArgs.GetThisArg()->GetEarlyNode();
GenTree* targetObjPointers = call->gtArgs.GetArgByIndex(1)->GetEarlyNode();
GenTree* thisPointer = call->gtArgs.GetThisArg()->GetNode();
GenTree* targetObjPointers = call->gtArgs.GetArgByIndex(1)->GetNode();
CORINFO_LOOKUP pLookup;
info.compCompHnd->getReadyToRunDelegateCtorHelper(&ldftnToken->m_token, ldftnToken->m_tokenConstraint,
clsHnd, &pLookup);
Expand Down Expand Up @@ -1175,8 +1176,8 @@ GenTree* Compiler::fgOptimizeDelegateConstructor(GenTreeCall* call,
{
JITDUMP("optimized\n");

GenTree* thisPointer = call->gtArgs.GetArgByIndex(0)->GetEarlyNode();
GenTree* targetObjPointers = call->gtArgs.GetArgByIndex(1)->GetEarlyNode();
GenTree* thisPointer = call->gtArgs.GetArgByIndex(0)->GetNode();
GenTree* targetObjPointers = call->gtArgs.GetArgByIndex(1)->GetNode();
call = gtNewHelperCallNode(CORINFO_HELP_READYTORUN_DELEGATE_CTOR, TYP_VOID, thisPointer, targetObjPointers);

CORINFO_LOOKUP entryPoint;
Expand Down Expand Up @@ -3852,10 +3853,8 @@ BasicBlock* Compiler::fgRngChkTarget(BasicBlock* block, SpecialCodeKind kind)
//
// Arguments:
// tree - the tree to sequence
// 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
// on all the nodes.
// isLIR - whether the sequencing is being done for LIR. If so, the
// GTF_REVERSE_OPS flag will be cleared on all nodes.
//
// Return Value:
// The first node to execute in the sequenced tree.
Expand Down Expand Up @@ -3887,12 +3886,6 @@ GenTree* Compiler::fgSetTreeSeq(GenTree* tree, bool isLIR)
if (m_isLIR)
{
node->ClearReverseOp();

// ARGPLACE nodes are not threaded into the LIR sequence.
if (node->OperIs(GT_ARGPLACE))
{
return fgWalkResult::WALK_CONTINUE;
}
}

node->gtPrev = m_prevNode;
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/forwardsub.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ class ForwardSubVisitor final : public GenTreeVisitor<ForwardSubVisitor>

for (CallArg& arg : m_callAncestor->gtArgs.Args())
{
m_useFlags |= (arg.GetEarlyNode()->gtFlags & GTF_GLOB_EFFECT);
m_useFlags |= (arg.GetNode()->gtFlags & GTF_GLOB_EFFECT);
}

if (oldUseFlags != m_useFlags)
Expand Down
Loading

0 comments on commit bb6954e

Please sign in to comment.