Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove GT_ARGPLACE nodes #68140

Merged
merged 7 commits into from
Apr 23, 2022
Merged

Remove GT_ARGPLACE nodes #68140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e88a36a
Remove GT_ARGPLACE nodes
jakobbotsch Apr 17, 2022
be8ed0c
Update docs
jakobbotsch Apr 17, 2022
df9fece
Merge branch 'main' of github.com:dotnet/runtime into remove-argplace…
jakobbotsch Apr 22, 2022
2983887
Remove stale comment
jakobbotsch Apr 22, 2022
9c26d6d
Merge branch 'main' of github.com:dotnet/runtime into remove-argplace…
jakobbotsch Apr 23, 2022
4bfbd7e
Fix merge
jakobbotsch Apr 23, 2022
5a6865a
Replace some GetEarlyNode uses with GetNode, fix some stale comments
jakobbotsch Apr 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions docs/design/coreclr/jit/jit-call-morphing.md
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was another instance of this and I also took the opportunity to change some uses of GetEarlyNode() to GetNode().

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
View file
Open in desktop
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