Skip to content

Commit

Permalink
JIT: Fix profiler tail call insertion logic for FIELD_LIST (#76883)
Browse files Browse the repository at this point in the history
This logic was not handling FIELD_LIST and was also not handling linear
order appropriately.

The logic is still a bit odd, it would probably be better to use the
same kind of logic as CFG (moving PUTARG nodes ahead of the profiler
hook instead), but in practice this seems to be ok.

Fix #76879
  • Loading branch information
jakobbotsch committed Oct 13, 2022
1 parent d2d5ad3 commit 8b999ee
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 29 deletions.
3 changes: 0 additions & 3 deletions src/coreclr/jit/codegenarmarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3069,9 +3069,6 @@ void CodeGen::genCall(GenTreeCall* call)
CallArgABIInformation& abiInfo = arg.AbiInfo;
GenTree* argNode = arg.GetLateNode();

// GT_RELOAD/GT_COPY use the child node
argNode = argNode->gtSkipReloadOrCopy();

if (abiInfo.GetRegNum() == REG_STK)
continue;

Expand Down
112 changes: 86 additions & 26 deletions src/coreclr/jit/lower.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1711,45 +1711,105 @@ void Lowering::InsertProfTailCallHook(GenTreeCall* call, GenTree* insertionPoint

if (insertionPoint == nullptr)
{
for (CallArg& arg : call->gtArgs.EarlyArgs())
{
assert(!arg.GetEarlyNode()->OperIs(GT_PUTARG_REG)); // We don't expect to see these in early args

if (arg.GetEarlyNode()->OperIs(GT_PUTARG_STK))
{
// found it
insertionPoint = arg.GetEarlyNode();
break;
}
}
insertionPoint = FindEarliestPutArg(call);

if (insertionPoint == nullptr)
{
for (CallArg& arg : call->gtArgs.LateArgs())
{
if (arg.GetLateNode()->OperIs(GT_PUTARG_REG, GT_PUTARG_STK))
{
// found it
insertionPoint = arg.GetLateNode();
break;
}
}

// If there are no args, insert before the call node
if (insertionPoint == nullptr)
{
insertionPoint = call;
}
insertionPoint = call;
}
}

#endif // !defined(TARGET_X86)

assert(insertionPoint != nullptr);
JITDUMP("Inserting profiler tail call before [%06u]\n", comp->dspTreeID(insertionPoint));

GenTree* profHookNode = new (comp, GT_PROF_HOOK) GenTree(GT_PROF_HOOK, TYP_VOID);
BlockRange().InsertBefore(insertionPoint, profHookNode);
}

//------------------------------------------------------------------------
// FindEarliestPutArg: Find the earliest direct PUTARG operand of a call node in
// linear order.
//
// Arguments:
// call - the call
//
// Returns:
// A PUTARG_* node that is the earliest of the call, or nullptr if the call
// has no arguments.
//
GenTree* Lowering::FindEarliestPutArg(GenTreeCall* call)
{
size_t numMarkedNodes = 0;
for (CallArg& arg : call->gtArgs.Args())
{
if (arg.GetEarlyNode() != nullptr)
{
numMarkedNodes += MarkPutArgNodes(arg.GetEarlyNode());
}

if (arg.GetLateNode() != nullptr)
{
numMarkedNodes += MarkPutArgNodes(arg.GetLateNode());
}
}

if (numMarkedNodes <= 0)
{
return nullptr;
}

GenTree* node = call;
do
{
node = node->gtPrev;

assert((node != nullptr) && "Reached beginning of basic block while looking for marked nodes");

if ((node->gtLIRFlags & LIR::Flags::Mark) != 0)
{
node->gtLIRFlags &= ~LIR::Flags::Mark;
numMarkedNodes--;
}
} while (numMarkedNodes > 0);

assert(node->OperIsPutArg());
return node;
}

//------------------------------------------------------------------------
// MarkPutArgNodes: Mark all direct operand PUTARG nodes with a LIR mark.
//
// Arguments:
// node - the node (either a field list or PUTARG node)
//
// Returns:
// The number of marks added.
//
size_t Lowering::MarkPutArgNodes(GenTree* node)
{
assert(node->OperIsPutArg() || node->OperIsFieldList());

size_t result = 0;
if (node->OperIsFieldList())
{
for (GenTreeFieldList::Use& operand : node->AsFieldList()->Uses())
{
assert(operand.GetNode()->OperIsPutArg());
result += MarkPutArgNodes(operand.GetNode());
}
}
else
{
assert((node->gtLIRFlags & LIR::Flags::Mark) == 0);
node->gtLIRFlags |= LIR::Flags::Mark;
result++;
}

return result;
}

//------------------------------------------------------------------------
// LowerFastTailCall: Lower a call node dispatched as a fast tailcall (epilog +
// jmp).
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/lower.h
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ class Lowering final : public Phase
GenTree* lookForUsesStart,
GenTreeCall* callNode);
void InsertProfTailCallHook(GenTreeCall* callNode, GenTree* insertionPoint);
GenTree* FindEarliestPutArg(GenTreeCall* call);
size_t MarkPutArgNodes(GenTree* node);
GenTree* LowerVirtualVtableCall(GenTreeCall* call);
GenTree* LowerVirtualStubCall(GenTreeCall* call);
void LowerArgsForCall(GenTreeCall* call);
Expand Down

0 comments on commit 8b999ee

Please sign in to comment.