Skip to content

JIT: Use precise multireg arg check for old promotion #112883

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.

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

Merged
merged 2 commits into from
Feb 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -4435,6 +4435,7 @@ class Compiler
void PromoteStructVar(unsigned lclNum);
void SortStructFields();
bool IsArmHfaParameter(unsigned lclNum);
bool IsSysVMultiRegType(ClassLayout* layout);

var_types TryPromoteValueClassAsPrimitive(CORINFO_TYPE_LAYOUT_NODE* treeNodes, size_t maxTreeNodes, size_t index);
void AdvanceSubTree(CORINFO_TYPE_LAYOUT_NODE* treeNodes, size_t maxTreeNodes, size_t* index);
Expand Down
28 changes: 26 additions & 2 deletions src/coreclr/jit/lclvars.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2634,6 +2634,28 @@ bool Compiler::StructPromotionHelper::IsArmHfaParameter(unsigned lclNum)
return hfaType != CORINFO_HFA_ELEM_NONE;
}

//--------------------------------------------------------------------------------------------
// IsSysVMultiRegType - Check if a type is one that could be passed in 2
// registers in some cases.
// This is a quirk to match old promotion behavior.
//
// Arguments:
// lclNum - The local
//
// Return value:
// True if it sometimes may be passed in two registers.
//
bool Compiler::StructPromotionHelper::IsSysVMultiRegType(ClassLayout* layout)
{
#ifdef UNIX_AMD64_ABI
Copy link
Member

Choose a reason for hiding this comment

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

not the case for arm64 (sysv/win)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not according to diffs... To be honest I did not spend too much time looking at why this turns out to be a good heuristic for SysV, since I hope to remove all of this in this or next release.

SYSTEMV_AMD64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR structDesc;
compiler->eeGetSystemVAmd64PassStructInRegisterDescriptor(layout->GetClassHandle(), &structDesc);
return structDesc.passedInRegisters && (structDesc.eightByteCount == 2);
#else
return false;
#endif
}

//--------------------------------------------------------------------------------------------
// ShouldPromoteStructVar - Should a struct var be promoted if it can be promoted?
// This routine mainly performs profitability checks. Right now it also has
Expand Down Expand Up @@ -2693,10 +2715,10 @@ bool Compiler::StructPromotionHelper::ShouldPromoteStructVar(unsigned lclNum)
#if FEATURE_MULTIREG_STRUCT_PROMOTE
// Is this a variable holding a value with exactly two fields passed in
// multiple registers?
if (compiler->lvaIsMultiregStruct(varDsc, compiler->info.compIsVarArgs))
if (varDsc->lvIsMultiRegArg || IsSysVMultiRegType(varDsc->GetLayout()))
{
if ((structPromotionInfo.fieldCnt != 2) &&
!((structPromotionInfo.fieldCnt == 1) && varTypeIsSIMD(structPromotionInfo.fields[0].fldType)))
((structPromotionInfo.fieldCnt != 1) || !varTypeIsSIMD(structPromotionInfo.fields[0].fldType)))
{
JITDUMP("Not promoting multireg struct local V%02u, because lvIsParam is true, #fields != 2 and it's "
"not a single SIMD.\n",
Expand All @@ -2713,6 +2735,7 @@ bool Compiler::StructPromotionHelper::ShouldPromoteStructVar(unsigned lclNum)
}
else
#endif // !FEATURE_MULTIREG_STRUCT_PROMOTE
{

// TODO-PERF - Implement struct promotion for incoming single-register structs.
// Also the implementation of jmp uses the 4 byte move to store
Expand All @@ -2726,6 +2749,7 @@ bool Compiler::StructPromotionHelper::ShouldPromoteStructVar(unsigned lclNum)
lclNum, structPromotionInfo.fieldCnt);
shouldPromote = false;
}
}
}
else if ((lclNum == compiler->genReturnLocal) && (structPromotionInfo.fieldCnt > 1))
{
Expand Down
Loading