Skip to content

Commit

Permalink
Remove GT_PUTARG_TYPE (#68748)
Browse files Browse the repository at this point in the history 
This change removes GT_PUTARG_TYPE by storing the signature type of the argument inside CallArg right away when arguments are added, instead of only from morph and on.
  • Loading branch information
@jakobbotsch
jakobbotsch authored May 9, 2022
1 parent 27f6e08 commit f8fa9f6
Show file tree
Hide file tree
Showing 13 changed files with 378 additions and 403 deletions.
12 changes: 4 additions & 8 deletions src/coreclr/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3911,8 +3911,10 @@ class Compiler

void impPopCallArgs(CORINFO_SIG_INFO* sig, GenTreeCall* call);

bool impCheckImplicitArgumentCoercion(var_types sigType, var_types nodeType) const;
public:
static bool impCheckImplicitArgumentCoercion(var_types sigType, var_types nodeType);

private:
void impPopReverseCallArgs(CORINFO_SIG_INFO* sig, GenTreeCall* call, unsigned skipReverseCount);

//---------------- Spilling the importer stack ----------------------------
Expand Down Expand Up @@ -4136,10 +4138,7 @@ class Compiler
InlineCandidateInfo** ppInlineCandidateInfo,
InlineResult* inlineResult);

void impInlineRecordArgInfo(InlineInfo* pInlineInfo,
GenTree* curArgVal,
unsigned argNum,
InlineResult* inlineResult);
void impInlineRecordArgInfo(InlineInfo* pInlineInfo, CallArg* arg, unsigned argNum, InlineResult* inlineResult);

void impInlineInitVars(InlineInfo* pInlineInfo);

Expand Down Expand Up @@ -4458,8 +4457,6 @@ class Compiler
BasicBlock* canonicalBlock,
flowList* predEdge);

GenTree* fgCheckCallArgUpdate(GenTree* parent, GenTree* child, var_types origType);

#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM)
// Sometimes we need to defer updating the BBF_FINALLY_TARGET bit. fgNeedToAddFinallyTargetBits signals
// when this is necessary.
Expand Down Expand Up @@ -10714,7 +10711,6 @@ class GenTreeVisitor
case GT_NULLCHECK:
case GT_PUTARG_REG:
case GT_PUTARG_STK:
case GT_PUTARG_TYPE:
case GT_RETURNTRAP:
case GT_NOP:
case GT_FIELD:
Expand Down
11 changes: 5 additions & 6 deletions src/coreclr/jit/compiler.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -991,11 +991,11 @@ inline GenTree* Compiler::gtNewIconEmbFldHndNode(CORINFO_FIELD_HANDLE fldHnd)
// Arguments:
// helper - Call helper
// type - Type of the node
// args - Call args
// args - Call args (struct args not supported)
//
// Return Value:
// New CT_HELPER node

//
inline GenTreeCall* Compiler::gtNewHelperCallNode(
unsigned helper, var_types type, GenTree* arg1, GenTree* arg2, GenTree* arg3)
{
Expand All @@ -1011,19 +1011,19 @@ inline GenTreeCall* Compiler::gtNewHelperCallNode(

if (arg3 != nullptr)
{
result->gtArgs.PushFront(this, arg3);
result->gtArgs.PushFront(this, NewCallArg::Primitive(arg3));
result->gtFlags |= arg3->gtFlags & GTF_ALL_EFFECT;
}

if (arg2 != nullptr)
{
result->gtArgs.PushFront(this, arg2);
result->gtArgs.PushFront(this, NewCallArg::Primitive(arg2));
result->gtFlags |= arg2->gtFlags & GTF_ALL_EFFECT;
}

if (arg1 != nullptr)
{
result->gtArgs.PushFront(this, arg1);
result->gtArgs.PushFront(this, NewCallArg::Primitive(arg1));
result->gtFlags |= arg1->gtFlags & GTF_ALL_EFFECT;
}

Expand Down Expand Up @@ -4265,7 +4265,6 @@ void GenTree::VisitOperands(TVisitor visitor)
case GT_NULLCHECK:
case GT_PUTARG_REG:
case GT_PUTARG_STK:
case GT_PUTARG_TYPE:
#if FEATURE_ARG_SPLIT
case GT_PUTARG_SPLIT:
#endif // FEATURE_ARG_SPLIT
Expand Down
9 changes: 5 additions & 4 deletions src/coreclr/jit/decomposelongs.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1355,10 +1355,11 @@ GenTree* DecomposeLongs::DecomposeShift(LIR::Use& use)
unreached();
}

GenTreeCall* call = m_compiler->gtNewHelperCallNode(helper, TYP_LONG);
call->gtArgs.PushFront(m_compiler, shiftByOp);
call->gtArgs.PushFront(m_compiler, hiOp1, WellKnownArg::ShiftHigh);
call->gtArgs.PushFront(m_compiler, loOp1, WellKnownArg::ShiftLow);
GenTreeCall* call = m_compiler->gtNewHelperCallNode(helper, TYP_LONG);
NewCallArg loArg = NewCallArg::Primitive(loOp1).WellKnown(WellKnownArg::ShiftLow);
NewCallArg hiArg = NewCallArg::Primitive(hiOp1).WellKnown(WellKnownArg::ShiftHigh);
NewCallArg shiftByArg = NewCallArg::Primitive(shiftByOp);
call->gtArgs.PushFront(m_compiler, loArg, hiArg, shiftByArg);
call->gtFlags |= shift->gtFlags & GTF_ALL_EFFECT;

if (shift->IsUnusedValue())
Expand Down
38 changes: 15 additions & 23 deletions src/coreclr/jit/fginline.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -540,15 +540,6 @@ Compiler::fgWalkResult Compiler::fgUpdateInlineReturnExpressionPlaceHolder(GenTr
JITDUMP("Updating type of the return GT_IND expression to TYP_BYREF\n");
inlineCandidate->gtType = TYP_BYREF;
}
else
{
// - under a call if we changed size of the argument.
GenTree* putArgType = comp->fgCheckCallArgUpdate(data->parent, inlineCandidate, retType);
if (putArgType != nullptr)
{
inlineCandidate = putArgType;
}
}
}

tree->ReplaceWith(inlineCandidate, comp);
Expand Down Expand Up @@ -824,14 +815,8 @@ Compiler::fgWalkResult Compiler::fgLateDevirtualization(GenTree** pTree, fgWalkD
{
const var_types retType = tree->TypeGet();
GenTree* foldedTree = comp->gtFoldExpr(tree);

GenTree* putArgType = comp->fgCheckCallArgUpdate(data->parent, foldedTree, retType);
if (putArgType != nullptr)
{
foldedTree = putArgType;
}
*pTree = foldedTree;
*madeChanges = true;
*pTree = foldedTree;
*madeChanges = true;
}

return WALK_CONTINUE;
Expand Down Expand Up @@ -1538,11 +1523,21 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo)
{
const InlArgInfo& argInfo = inlArgInfo[argNum];
const bool argIsSingleDef = !argInfo.argHasLdargaOp && !argInfo.argHasStargOp;
GenTree* argNode = inlArgInfo[argNum].argNode;
const bool argHasPutArg = argNode->OperIs(GT_PUTARG_TYPE);
CallArg* arg = argInfo.arg;
GenTree* argNode = arg->GetNode();

// TODO-ARGS: This can probably be relaxed, the old comment was:
// argHasPutArg disqualifies the arg from a direct substitution because we don't have information about
// its user. For example: replace `LCL_VAR short` with `PUTARG_TYPE short->LCL_VAR int`,
// we should keep `PUTARG_TYPE` iff the user is a call that needs `short` and delete it otherwise.
//
// Today we no longer have this contextual PUTARG_TYPE and morph
// should properly handle substituting a TYP_INT node for a
// TYP_SHORT LCL_VAR (at least for a call arg).
bool argHasPutArg = !varTypeIsStruct(arg->GetSignatureType()) &&
(genTypeSize(argNode) != genTypeSize(arg->GetSignatureType()));

BasicBlockFlags bbFlags = BBF_EMPTY;
argNode = argNode->gtSkipPutArgType();
argNode = argNode->gtRetExprVal(&bbFlags);

if (argInfo.argHasTmp)
Expand All @@ -1563,9 +1558,6 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo)

GenTree* argSingleUseNode = argInfo.argBashTmpNode;

// argHasPutArg disqualifies the arg from a direct substitution because we don't have information about
// its user. For example: replace `LCL_VAR short` with `PUTARG_TYPE short->LCL_VAR int`,
// we should keep `PUTARG_TYPE` iff the user is a call that needs `short` and delete it otherwise.
if ((argSingleUseNode != nullptr) && !(argSingleUseNode->gtFlags & GTF_VAR_CLONED) && argIsSingleDef &&
!argHasPutArg)
{
Expand Down
44 changes: 3 additions & 41 deletions src/coreclr/jit/flowgraph.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1216,19 +1216,19 @@ GenTree* Compiler::fgOptimizeDelegateConstructor(GenTreeCall* call,
if (ctorData.pArg3 != nullptr)
{
GenTree* arg3 = gtNewIconHandleNode(size_t(ctorData.pArg3), GTF_ICON_FTN_ADDR);
lastArg = call->gtArgs.PushBack(this, arg3);
lastArg = call->gtArgs.PushBack(this, NewCallArg::Primitive(arg3));
}

if (ctorData.pArg4 != nullptr)
{
GenTree* arg4 = gtNewIconHandleNode(size_t(ctorData.pArg4), GTF_ICON_FTN_ADDR);
lastArg = call->gtArgs.InsertAfter(this, lastArg, arg4);
lastArg = call->gtArgs.InsertAfter(this, lastArg, NewCallArg::Primitive(arg4));
}

if (ctorData.pArg5 != nullptr)
{
GenTree* arg5 = gtNewIconHandleNode(size_t(ctorData.pArg5), GTF_ICON_FTN_ADDR);
lastArg = call->gtArgs.InsertAfter(this, lastArg, arg5);
lastArg = call->gtArgs.InsertAfter(this, lastArg, NewCallArg::Primitive(arg5));
}
}
else
Expand Down Expand Up @@ -4186,41 +4186,3 @@ void Compiler::fgLclFldAssign(unsigned lclNum)
lvaSetVarDoNotEnregister(lclNum DEBUGARG(DoNotEnregisterReason::LocalField));
}
}

//------------------------------------------------------------------------
// fgCheckCallArgUpdate: check if we are replacing a call argument and add GT_PUTARG_TYPE if necessary.
//
// Arguments:
// parent - the parent that could be a call;
// child - the new child node;
// origType - the original child type;
//
// Returns:
// PUT_ARG_TYPE node if it is needed, nullptr otherwise.
//
GenTree* Compiler::fgCheckCallArgUpdate(GenTree* parent, GenTree* child, var_types origType)
{
if ((parent == nullptr) || !parent->IsCall())
{
return nullptr;
}
const var_types newType = child->TypeGet();
if (newType == origType)
{
return nullptr;
}
if (varTypeIsStruct(origType) || (genTypeSize(origType) == genTypeSize(newType)))
{
assert(!varTypeIsStruct(newType));
return nullptr;
}
GenTree* putArgType = gtNewOperNode(GT_PUTARG_TYPE, origType, child);
#if defined(DEBUG)
if (verbose)
{
printf("For call [%06d] the new argument's type [%06d]", dspTreeID(parent), dspTreeID(child));
printf(" does not match the original type size, add a GT_PUTARG_TYPE [%06d]\n", dspTreeID(parent));
}
#endif
return putArgType;
}
Loading

0 comments on commit f8fa9f6

Please sign in to comment.