Skip to content

Commit

Permalink
Enable TYP_STRUCT LCL_VAR/LCL_FLD call args on Unix x64 (dotnet#7…
Browse files Browse the repository at this point in the history 
…0960)

* Unix x64: local morph

* Morph and costs tuning

Required to avoid regressions due to args sorting.
  • Loading branch information
@SingleAccretion
SingleAccretion authored Jun 23, 2022
1 parent c299002 commit 34d5286
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 22 deletions.
26 changes: 15 additions & 11 deletions src/coreclr/jit/gentree.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4654,12 +4654,18 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree)
{
costEx = IND_COST_EX;
costSz = 2;
/* Sign-extend and zero-extend are more expensive to load */

// Some types are more expensive to load than others.
if (varTypeIsSmall(tree->TypeGet()))
{
costEx += 1;
costSz += 1;
}
else if (tree->TypeIs(TYP_STRUCT))
{
costEx += 2 * IND_COST_EX;
costSz += 2 * 2;
}
}
#if defined(TARGET_AMD64)
// increase costSz for floating point locals
Expand All @@ -4685,8 +4691,8 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree)
}
else if (tree->TypeIs(TYP_STRUCT))
{
costEx += IND_COST_EX;
costSz += 2;
costEx += 2 * IND_COST_EX;
costSz += 2 * 2;
}
break;

Expand Down Expand Up @@ -4916,17 +4922,15 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree)
break;

case GT_ADDR:
if (op1->OperIsLocalRead())
{
costEx = 3;
costSz = 3;
goto DONE;
}

costEx = 0;
costSz = 1;

// If we have a GT_ADDR of an GT_IND we can just copy the costs from indOp1
if (op1->OperGet() == GT_IND)
{
GenTree* indOp1 = op1->AsOp()->gtOp1;
costEx = indOp1->GetCostEx();
costSz = indOp1->GetCostSz();
}
break;

case GT_ARR_LENGTH:
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/jit/lclmorph.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1095,7 +1095,7 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
// | Partial | LCL_FLD | OBJ/LCL_FLD | LCL_FLD |
// |------------|---------|-------------|---------|
//
// * - On x86/Windows x64 only.
// * - On XArch only.
//
// |------------|------|------|--------|----------|
// | SIMD | CALL | ASG | RETURN | HWI/SIMD |
Expand All @@ -1113,9 +1113,9 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>

if (user->IsCall())
{
#if !defined(WINDOWS_AMD64_ABI) && !defined(TARGET_X86)
#if !defined(TARGET_XARCH)
return IndirTransform::None;
#endif // !defined(WINDOWS_AMD64_ABI) && !defined(TARGET_X86)
#endif // !defined(TARGET_XARCH)
}

if (match == StructMatch::Compatible)
Expand Down
19 changes: 11 additions & 8 deletions src/coreclr/jit/morph.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1364,7 +1364,8 @@ void CallArgs::SortArgs(Compiler* comp, GenTreeCall* call, CallArg** sortedArgs)
GenTree* argx = arg->GetEarlyNode();
assert(argx != nullptr);

if ((argx->gtOper == GT_LCL_VAR) || (argx->gtOper == GT_LCL_FLD))
// As a CQ heuristic, sort TYP_STRUCT args using the cost estimation below.
if (!argx->TypeIs(TYP_STRUCT) && argx->OperIs(GT_LCL_VAR, GT_LCL_FLD))
{
noway_assert(curInx <= endTab);

Expand Down Expand Up @@ -1410,9 +1411,8 @@ void CallArgs::SortArgs(Compiler* comp, GenTreeCall* call, CallArg** sortedArgs)
assert(argx != nullptr);

// We should have already handled these kinds of args
assert(argx->gtOper != GT_LCL_VAR);
assert(argx->gtOper != GT_LCL_FLD);
assert(argx->gtOper != GT_CNS_INT);
assert((!argx->OperIs(GT_LCL_VAR, GT_LCL_FLD) || argx->TypeIs(TYP_STRUCT)) &&
!argx->OperIs(GT_CNS_INT));

// This arg should either have no persistent side effects or be the last one in our table
// assert(((argx->gtFlags & GTF_PERSISTENT_SIDE_EFFECTS) == 0) || (curInx == (argCount-1)));
Expand Down Expand Up @@ -3261,6 +3261,7 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call)
// - This is irrelevant for X86, since structs are always passed by value on the stack.

GenTree* lclVar = fgIsIndirOfAddrOfLocal(argObj);
bool argIsLocal = (lclVar != nullptr) || argObj->OperIsLocalRead();
bool canTransform = false;

if (structBaseType != TYP_STRUCT)
Expand All @@ -3278,7 +3279,7 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call)
// or UNIX_AMD64_ABI cases where they will be passed in registers.
else
{
canTransform = (lclVar != nullptr);
canTransform = argIsLocal;
passingSize = genTypeSize(structBaseType);
}
#endif // TARGET_ARM64 || UNIX_AMD64_ABI || TARGET_LOONGARCH64
Expand Down Expand Up @@ -3308,7 +3309,7 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call)
makeOutArgCopy = true;
}
}
else if (lclVar == nullptr)
else if (!argIsLocal)
{
// This should only be the case of a value directly producing a known struct type.
assert(argObj->TypeGet() != TYP_STRUCT);
Expand Down Expand Up @@ -3786,18 +3787,20 @@ GenTree* Compiler::fgMorphMultiregStructArg(CallArg* arg)
{
argNode = fgMorphLclArgToFieldlist(lcl);
}
#ifndef TARGET_XARCH
else if (argNode->TypeGet() == TYP_STRUCT)
{
// If this is a non-register struct, it must be referenced from memory.
// ARM/ARM64/LoongArch64 backends do not support local nodes as sources of some stack args.
if (!actualArg->OperIs(GT_OBJ))
{
// Create an Obj of the temp to use it as a call argument.
argNode = gtNewOperNode(GT_ADDR, TYP_I_IMPL, argNode);
argNode = gtNewObjNode(lvaGetStruct(lcl->GetLclNum()), argNode);
}
// Its fields will need to be accessed by address.
lvaSetVarDoNotEnregister(lcl->GetLclNum() DEBUG_ARG(DoNotEnregisterReason::IsStructArg));
lvaSetVarDoNotEnregister(lcl->GetLclNum() DEBUGARG(DoNotEnregisterReason::IsStructArg));
}
#endif // !TARGET_XARCH
}

return argNode;
Expand Down

0 comments on commit 34d5286

Please sign in to comment.