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

JIT: Remove CallArg::m_tmpNum #104429

Merged
merged 1 commit into from
Jul 10, 2024
+69 −119
Merged

JIT: Remove CallArg::m_tmpNum #104429

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 0 additions & 2 deletions src/coreclr/jit/gentree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9738,12 +9738,10 @@ void CallArgs::InternalCopyFrom(Compiler* comp, CallArgs* other, CopyNodeFunc co
carg->m_earlyNode = arg.m_earlyNode != nullptr ? copyNode(arg.m_earlyNode) : nullptr;
carg->m_lateNode = arg.m_lateNode != nullptr ? copyNode(arg.m_lateNode) : nullptr;
carg->m_signatureClsHnd = arg.m_signatureClsHnd;
carg->m_tmpNum = arg.m_tmpNum;
carg->m_signatureType = arg.m_signatureType;
carg->m_wellKnownArg = arg.m_wellKnownArg;
carg->m_needTmp = arg.m_needTmp;
carg->m_needPlace = arg.m_needPlace;
carg->m_isTmp = arg.m_isTmp;
carg->m_processed = arg.m_processed;
carg->AbiInfo = arg.AbiInfo;
carg->NewAbiInfo = arg.NewAbiInfo;
Expand Down
10 changes: 1 addition & 9 deletions src/coreclr/jit/gentree.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4708,8 +4708,6 @@ class CallArg

// The class handle for the signature type (when varTypeIsStruct(SignatureType)).
CORINFO_CLASS_HANDLE m_signatureClsHnd;
// The LclVar number if we had to force evaluation of this arg.
unsigned m_tmpNum;
// The type of the argument in the signature.
var_types m_signatureType : 5;
// The type of well-known argument this is.
Expand All @@ -4718,8 +4716,6 @@ class CallArg
bool m_needTmp : 1;
// True when we must replace this argument with a placeholder node.
bool m_needPlace : 1;
// True when we setup a temp LclVar for this argument.
bool m_isTmp : 1;
// True when we have decided the evaluation order for this argument in LateArgs
bool m_processed : 1;

Expand All @@ -4730,12 +4726,10 @@ class CallArg
, m_next(nullptr)
, m_lateNext(nullptr)
, m_signatureClsHnd(NO_CLASS_HANDLE)
, m_tmpNum(BAD_VAR_NUM)
, m_signatureType(TYP_UNDEF)
, m_wellKnownArg(WellKnownArg::None)
, m_needTmp(false)
, m_needPlace(false)
, m_isTmp(false)
, m_processed(false)
{
}
Expand Down Expand Up @@ -4772,7 +4766,6 @@ class CallArg
CORINFO_CLASS_HANDLE GetSignatureClassHandle() { return m_signatureClsHnd; }
var_types GetSignatureType() { return m_signatureType; }
WellKnownArg GetWellKnownArg() { return m_wellKnownArg; }
bool IsTemp() { return m_isTmp; }
// clang-format on

// Get the real argument node, i.e. not a setup or placeholder node.
Expand Down Expand Up @@ -4884,8 +4877,7 @@ class CallArgs
void SetNeedsTemp(CallArg* arg);
bool IsNonStandard(Compiler* comp, GenTreeCall* call, CallArg* arg);

GenTree* MakeTmpArgNode(Compiler* comp, CallArg* arg);
void SetTemp(CallArg* arg, unsigned tmpNum);
GenTree* MakeTmpArgNode(Compiler* comp, CallArg* arg, unsigned lclNum);

// clang-format off
bool HasThisPointer() const { return m_hasThisPointer; }
Expand Down
176 changes: 68 additions & 108 deletions src/coreclr/jit/morph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -821,18 +821,10 @@ void CallArg::Dump(Compiler* comp)
{
printf(", isSplit");
}
if (m_needTmp)
{
printf(", tmpNum=V%02u", m_tmpNum);
}
if (m_needPlace)
{
printf(", needPlace");
}
if (m_isTmp)
{
printf(", isTmp");
}
if (m_processed)
{
printf(", processed");
Expand All @@ -849,15 +841,6 @@ void CallArg::Dump(Compiler* comp)
}
#endif

//------------------------------------------------------------------------
// SetTemp: Set that the specified argument was evaluated into a temp.
//
void CallArgs::SetTemp(CallArg* arg, unsigned tmpNum)
{
arg->m_tmpNum = tmpNum;
arg->m_isTmp = true;
}

//------------------------------------------------------------------------
// ArgsComplete: Make final decisions on which arguments to evaluate into temporaries.
//
Expand All @@ -874,13 +857,7 @@ void CallArgs::ArgsComplete(Compiler* comp, GenTreeCall* call)
for (CallArg& arg : Args())
{
GenTree* argx = arg.GetEarlyNode();

if (argx == nullptr)
{
// Should only happen if remorphing in which case we do not need to
// make a decision about temps.
continue;
}
Comment on lines -877 to -883
Copy link
Member Author

Choose a reason for hiding this comment

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

ArgsComplete is not called during remorphing, so the code should be dead.

assert(argx != nullptr);

bool canEvalToTemp = true;
if (arg.AbiInfo.GetRegNum() == REG_STK)
Expand Down Expand Up @@ -909,19 +886,16 @@ void CallArgs::ArgsComplete(Compiler* comp, GenTreeCall* call)
//
if ((argx->gtFlags & GTF_ASG) != 0)
{
// If this is not the only argument, or it's a copyblk, or it
// already evaluates the expression to a tmp then we need a temp in
// the late arg list.
// In the latter case this might not even be a value;
// fgMakeOutgoingStructArgCopy will leave the copying nodes here
// for FEATURE_FIXED_OUT_ARGS.
if (canEvalToTemp && ((argCount > 1) || argx->OperIsCopyBlkOp() || (FEATURE_FIXED_OUT_ARGS && arg.m_isTmp)))
// fgMakeOutgoingStructArgCopy can have introduced a temp already,
// in which case it will have created a setup node in the early
// node.
if (!argx->IsValue())
{
SetNeedsTemp(&arg);
assert(arg.m_needTmp);
}
else
else if (canEvalToTemp && (argCount > 1))
{
assert(argx->IsValue());
SetNeedsTemp(&arg);
}

// For all previous arguments that may interfere with the store we
Expand Down Expand Up @@ -1318,8 +1292,6 @@ void CallArgs::SortArgs(Compiler* comp, GenTreeCall* call, CallArg** sortedArgs)
regCount++;
}

assert(arg->GetLateNode() == nullptr);

// Skip any already processed args
//
if (!arg->m_processed)
Expand Down Expand Up @@ -1556,9 +1528,8 @@ void CallArgs::SortArgs(Compiler* comp, GenTreeCall* call, CallArg** sortedArgs)
// Return Value:
// the newly created temp var tree.
//
GenTree* CallArgs::MakeTmpArgNode(Compiler* comp, CallArg* arg)
GenTree* CallArgs::MakeTmpArgNode(Compiler* comp, CallArg* arg, unsigned lclNum)
{
unsigned lclNum = arg->m_tmpNum;
LclVarDsc* varDsc = comp->lvaGetDesc(lclNum);
var_types argType = varDsc->TypeGet();
assert(genActualType(argType) == genActualType(arg->GetSignatureType()));
Expand Down Expand Up @@ -1633,7 +1604,16 @@ void CallArgs::EvalArgsToTemps(Compiler* comp, GenTreeCall* call)
for (size_t i = 0; i < numArgs; i++)
{
CallArg& arg = *(sortedArgs[i]);
assert(arg.GetLateNode() == nullptr);

if (arg.GetLateNode() != nullptr)
{
// We may already have created the temp as part of
// fgMakeOutgoingStructArgCopy. In that case there is no work to be
// done.
*lateTail = &arg;
lateTail = &arg.LateNextRef();
continue;
}

GenTree* argx = arg.GetEarlyNode();
assert(argx != nullptr);
Expand All @@ -1655,82 +1635,60 @@ void CallArgs::EvalArgsToTemps(Compiler* comp, GenTreeCall* call)

if (arg.m_needTmp)
{
if (arg.m_isTmp)
{
// Create a copy of the temp to go into the late argument list
defArg = MakeTmpArgNode(comp, &arg);
}
else
{
// Create a temp store for the argument
// Put the temp in the late arg list
// Create a temp store for the argument
// Put the temp in the late arg list

#ifdef DEBUG
if (comp->verbose)
{
printf("Argument with 'side effect'...\n");
comp->gtDispTree(argx);
}
if (comp->verbose)
{
printf("Argument with 'side effect'...\n");
comp->gtDispTree(argx);
}
#endif

#if defined(TARGET_AMD64) && !defined(UNIX_AMD64_ABI)
noway_assert(argx->gtType != TYP_STRUCT);
noway_assert(argx->gtType != TYP_STRUCT);
#endif

unsigned tmpVarNum = comp->lvaGrabTemp(true DEBUGARG("argument with side effect"));
unsigned tmpVarNum = comp->lvaGrabTemp(true DEBUGARG("argument with side effect"));

if (setupArg != nullptr)
{
// Now keep the mkrefany for the late argument list
defArg = argx;
setupArg = comp->gtNewTempStore(tmpVarNum, argx);

// Clear the side-effect flags because now both op1 and op2 have no side-effects
defArg->gtFlags &= ~GTF_ALL_EFFECT;
}
else
{
setupArg = comp->gtNewTempStore(tmpVarNum, argx);
LclVarDsc* varDsc = comp->lvaGetDesc(tmpVarNum);
var_types lclVarType = genActualType(argx->gtType);
var_types scalarType = TYP_UNKNOWN;

LclVarDsc* varDsc = comp->lvaGetDesc(tmpVarNum);
var_types lclVarType = genActualType(argx->gtType);
var_types scalarType = TYP_UNKNOWN;

if (setupArg->OperIsCopyBlkOp())
{
setupArg = comp->fgMorphCopyBlock(setupArg);
if (setupArg->OperIsCopyBlkOp())
{
setupArg = comp->fgMorphCopyBlock(setupArg);
#if defined(TARGET_ARMARCH) || defined(UNIX_AMD64_ABI) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64)
if ((lclVarType == TYP_STRUCT) && (arg.AbiInfo.ArgType != TYP_STRUCT))
{
scalarType = arg.AbiInfo.ArgType;
}
if ((lclVarType == TYP_STRUCT) && (arg.AbiInfo.ArgType != TYP_STRUCT))
{
scalarType = arg.AbiInfo.ArgType;
}
#endif // TARGET_ARMARCH || defined (UNIX_AMD64_ABI) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64)
}

// scalarType can be set to a wider type for ARM or unix amd64 architectures: (3 => 4) or (5,6,7 =>
// 8)
if ((scalarType != TYP_UNKNOWN) && (scalarType != lclVarType))
{
// Create a GT_LCL_FLD using the wider type to go to the late argument list
defArg = comp->gtNewLclFldNode(tmpVarNum, scalarType, 0);
}
else
{
// Create a copy of the temp to go to the late argument list
defArg = comp->gtNewLclvNode(tmpVarNum, lclVarType);
}
}

arg.m_isTmp = true;
arg.m_tmpNum = tmpVarNum;
}
// scalarType can be set to a wider type for ARM or unix amd64 architectures: (3 => 4) or (5,6,7 =>
// 8)
if ((scalarType != TYP_UNKNOWN) && (scalarType != lclVarType))
{
// Create a GT_LCL_FLD using the wider type to go to the late argument list
defArg = comp->gtNewLclFldNode(tmpVarNum, scalarType, 0);
}
else
{
// Create a copy of the temp to go to the late argument list
defArg = comp->gtNewLclvNode(tmpVarNum, lclVarType);
}

#ifdef DEBUG
if (comp->verbose)
{
printf("\n Evaluate to a temp:\n");
comp->gtDispTree(setupArg);
}
#endif
if (comp->verbose)
{
printf("\n Evaluate to a temp:\n");
comp->gtDispTree(setupArg);
}
#endif
}
else // curArgTabEntry->needTmp == false
{
Expand Down Expand Up @@ -3264,29 +3222,31 @@ void Compiler::fgMakeOutgoingStructArgCopy(GenTreeCall* call, CallArg* arg)
assert(!fgGlobalMorph);
}

call->gtArgs.SetNeedsTemp(arg);
Copy link
Member Author

Choose a reason for hiding this comment

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

This was previously done in ArgsComplete (the code in the GTF_ASG case), but it makes more sense just to do it immediately when we introduce the temp.


// Copy the valuetype to the temp
GenTree* copyBlk = gtNewStoreLclVarNode(tmp, argx);
copyBlk = fgMorphCopyBlock(copyBlk);

call->gtArgs.SetTemp(arg, tmp);
#if FEATURE_FIXED_OUT_ARGS

// Do the copy early, and evaluate the temp later (see EvalArgsToTemps)
// When on Unix create LCL_FLD for structs passed in more than one registers. See fgMakeTmpArgNode
GenTree* argNode = copyBlk;
// For fixed out args we create the setup node here; EvalArgsToTemps knows
// to handle the case of "already have a setup node" properly.
arg->SetEarlyNode(copyBlk);
arg->SetLateNode(call->gtArgs.MakeTmpArgNode(this, arg, tmp));

#else // !FEATURE_FIXED_OUT_ARGS

// Structs are always on the stack, and thus never need temps
// so we have to put the copy and temp all into one expression.
GenTree* argNode = call->gtArgs.MakeTmpArgNode(this, arg);
GenTree* argNode = call->gtArgs.MakeTmpArgNode(this, arg, tmp);

// Change the expression to "(tmp=val),tmp"
argNode = gtNewOperNode(GT_COMMA, argNode->TypeGet(), copyBlk, argNode);

#endif // !FEATURE_FIXED_OUT_ARGS

arg->SetEarlyNode(argNode);

#endif // !FEATURE_FIXED_OUT_ARGS
}

/*****************************************************************************
Expand Down Expand Up @@ -6747,12 +6707,12 @@ Statement* Compiler::fgAssignRecursiveCallArgToCallerParam(GenTree* arg,
// TODO-CQ: enable calls with struct arguments passed in registers.
noway_assert(!varTypeIsStruct(arg->TypeGet()));

if (callArg->IsTemp() || arg->IsCnsIntOrI() || arg->IsCnsFltOrDbl())
if (arg->IsCnsIntOrI() || arg->IsCnsFltOrDbl())
{
// The argument is already assigned to a temp or is a const.
argInTemp = arg;
}
else if (arg->OperGet() == GT_LCL_VAR)
else if (arg->OperIs(GT_LCL_VAR))
{
unsigned lclNum = arg->AsLclVar()->GetLclNum();
LclVarDsc* varDsc = lvaGetDesc(lclNum);
Expand Down
Loading