Skip to content

Commit

Permalink
Use the signature types when building ABI info (#70635)
Browse files Browse the repository at this point in the history 
* Use signature types when building the ABI info

This will allow us to let "mismatched" struct types as call arguments,
which in turn is expected to simplify and de-pessimize much of the code
working around the need to keep precise handles on struct nodes.

Credit to @jakobbotsch for being able to make this change.

* Work around the "InferOpSizeAlign" problem

Handling it generally requires solving a larger problem
of "eeGetArgSizeAlignment" not working well on ARM.

(See also the use of "eeGetArgSizeAlignment" in "lvaInitUserArgs")
  • Loading branch information
@SingleAccretion
SingleAccretion committed Jun 13, 2022
1 parent 098b142 commit 6418252
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 88 deletions.
29 changes: 0 additions & 29 deletions src/coreclr/jit/codegencommon.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6581,35 +6581,6 @@ bool Compiler::IsHfa(CORINFO_CLASS_HANDLE hClass)
return varTypeIsValidHfaType(GetHfaType(hClass));
}

bool Compiler::IsHfa(GenTree* tree)
{
if (GlobalJitOptions::compFeatureHfa)
{
return IsHfa(gtGetStructHandleIfPresent(tree));
}
else
{
return false;
}
}

var_types Compiler::GetHfaType(GenTree* tree)
{
if (GlobalJitOptions::compFeatureHfa)
{
return GetHfaType(gtGetStructHandleIfPresent(tree));
}
else
{
return TYP_UNDEF;
}
}

unsigned Compiler::GetHfaCount(GenTree* tree)
{
return GetHfaCount(gtGetStructHandle(tree));
}

var_types Compiler::GetHfaType(CORINFO_CLASS_HANDLE hClass)
{
if (GlobalJitOptions::compFeatureHfa)
Expand Down
5 changes: 0 additions & 5 deletions src/coreclr/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1882,11 +1882,6 @@ class Compiler
//

bool IsHfa(CORINFO_CLASS_HANDLE hClass);
bool IsHfa(GenTree* tree);

var_types GetHfaType(GenTree* tree);
unsigned GetHfaCount(GenTree* tree);

var_types GetHfaType(CORINFO_CLASS_HANDLE hClass);
unsigned GetHfaCount(CORINFO_CLASS_HANDLE hClass);

Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/jit/lower.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3814,18 +3814,18 @@ void Lowering::LowerCallStruct(GenTreeCall* call)

if (GlobalJitOptions::compFeatureHfa)
{
if (comp->IsHfa(call))
if (comp->IsHfa(call->gtRetClsHnd))
{
#if defined(TARGET_ARM64)
assert(comp->GetHfaCount(call) == 1);
assert(comp->GetHfaCount(call->gtRetClsHnd) == 1);
#elif defined(TARGET_ARM)
// ARM returns double in 2 float registers, but
// `call->HasMultiRegRetVal()` count double registers.
assert(comp->GetHfaCount(call) <= 2);
assert(comp->GetHfaCount(call->gtRetClsHnd) <= 2);
#else // !TARGET_ARM64 && !TARGET_ARM
NYI("Unknown architecture");
#endif // !TARGET_ARM64 && !TARGET_ARM
var_types hfaType = comp->GetHfaType(call);
var_types hfaType = comp->GetHfaType(call->gtRetClsHnd);
if (call->TypeIs(hfaType))
{
return;
Expand Down
100 changes: 50 additions & 50 deletions src/coreclr/jit/morph.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2233,7 +2233,12 @@ void CallArgs::AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call
argx->gtType = TYP_I_IMPL;
}

// Setup any HFA information about 'argx'
// Note we must use the signature types for making ABI decisions. This is especially important for structs,
// where the "argx" node can legally have a type that is not ABI-compatible with the one in the signature.
const var_types argSigType = arg.GetSignatureType();
const CORINFO_CLASS_HANDLE argSigClass = arg.GetSignatureClassHandle();

// Setup any HFA information about the argument.
bool isHfaArg = false;
var_types hfaType = TYP_UNDEF;
unsigned hfaSlots = 0;
Expand All @@ -2245,7 +2250,7 @@ void CallArgs::AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call

if (GlobalJitOptions::compFeatureHfa)
{
hfaType = comp->GetHfaType(argx);
hfaType = comp->GetHfaType(argSigClass);
isHfaArg = varTypeIsValidHfaType(hfaType);

#if defined(TARGET_ARM64)
Expand All @@ -2258,7 +2263,7 @@ void CallArgs::AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call

if (isHfaArg)
{
hfaSlots = comp->GetHfaCount(argx);
hfaSlots = comp->GetHfaCount(argSigClass);

// If we have a HFA struct it's possible we transition from a method that originally
// only had integer types to now start having FP types. We have to communicate this
Expand All @@ -2272,11 +2277,19 @@ void CallArgs::AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call
const bool isFloatHfa = (hfaType == TYP_FLOAT);

#ifdef TARGET_ARM
passUsingFloatRegs = !callIsVararg && (isHfaArg || varTypeUsesFloatReg(argx)) && !comp->opts.compUseSoftFP;
passUsingFloatRegs =
!callIsVararg && (isHfaArg || varTypeUsesFloatReg(argSigType)) && !comp->opts.compUseSoftFP;
bool passUsingIntRegs = passUsingFloatRegs ? false : (intArgRegNum < MAX_REG_ARG);

// We don't use the "size" return value from InferOpSizeAlign().
comp->codeGen->InferOpSizeAlign(argx, &argAlignBytes);
// TODO-Cleanup: use "eeGetArgSizeAlignment" here. See also: https://github.com/dotnet/runtime/issues/46026.
if (varTypeIsStruct(argSigType))
{
argAlignBytes = comp->info.compCompHnd->getClassAlignmentRequirement(argSigClass);
}
else
{
argAlignBytes = genTypeSize(argSigType);
}

argAlignBytes = roundUp(argAlignBytes, TARGET_POINTER_SIZE);

Expand All @@ -2303,11 +2316,11 @@ void CallArgs::AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call
#elif defined(TARGET_ARM64)

assert(!callIsVararg || !isHfaArg);
passUsingFloatRegs = !callIsVararg && (isHfaArg || varTypeUsesFloatReg(argx));
passUsingFloatRegs = !callIsVararg && (isHfaArg || varTypeUsesFloatReg(argSigType));

#elif defined(TARGET_AMD64)

passUsingFloatRegs = varTypeIsFloating(argx);
passUsingFloatRegs = varTypeIsFloating(argSigType);

#elif defined(TARGET_X86)

Expand All @@ -2316,7 +2329,7 @@ void CallArgs::AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call
#elif defined(TARGET_LOONGARCH64)

assert(!callIsVararg && !isHfaArg);
passUsingFloatRegs = varTypeUsesFloatReg(argx);
passUsingFloatRegs = varTypeUsesFloatReg(argSigType);
DWORD floatFieldFlags = STRUCT_NO_FLOAT_FIELD;

#else
Expand All @@ -2325,43 +2338,34 @@ void CallArgs::AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call

bool isBackFilled = false;
unsigned nextFltArgRegNum = fltArgRegNum; // This is the next floating-point argument register number to use
bool isStructArg = varTypeIsStruct(argSigType);
var_types structBaseType = TYP_STRUCT;
unsigned structSize = 0;
bool passStructByRef = false;

bool isStructArg;
GenTree* actualArg = argx->gtEffectiveVal(true /* Commas only */);

//
// Figure out the size of the argument. This is either in number of registers, or number of
// TARGET_POINTER_SIZE stack slots, or the sum of these if the argument is split between the registers and
// the stack.
//
isStructArg = varTypeIsStruct(argx);
// Note that we internally in the JIT can change some struct args to
// primitive args (e.g. OBJ<struct>(x) -> IND<TYP_LONG>(x)). Similarly,
// the ABI type can also change from struct to primitive (e.g. a 8-byte
// struct passed in a register). So isStructArg may be false even if
// the signature type was (or is) a struct, however only in cases where
// it does not matter.
CORINFO_CLASS_HANDLE objClass = NO_CLASS_HANDLE;

if (isStructArg)
{
objClass = comp->gtGetStructHandle(argx);
if (argx->TypeGet() == TYP_STRUCT)
GenTree* actualArg = argx->gtEffectiveVal(true /* Commas only */);

// Here we look at "actualArg" to avoid calling "getClassSize".
if (actualArg->TypeGet() == TYP_STRUCT)
{
// For TYP_STRUCT arguments we must have an OBJ, LCL_VAR or MKREFANY
switch (actualArg->OperGet())
{
case GT_OBJ:
structSize = actualArg->AsObj()->GetLayout()->GetSize();
assert(structSize == comp->info.compCompHnd->getClassSize(objClass));
structSize = actualArg->AsObj()->Size();
break;
case GT_LCL_VAR:
structSize = comp->lvaGetDesc(actualArg->AsLclVarCommon())->lvExactSize;
break;
case GT_MKREFANY:
structSize = comp->info.compCompHnd->getClassSize(objClass);
structSize = comp->info.compCompHnd->getClassSize(argSigClass);
break;
default:
BADCODE("illegal argument tree: cannot determine size for ABI handling");
Expand All @@ -2370,28 +2374,29 @@ void CallArgs::AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call
}
else
{
structSize = genTypeSize(argx);
assert(structSize == comp->info.compCompHnd->getClassSize(objClass));
structSize = genTypeSize(actualArg);
}

assert(structSize = comp->info.compCompHnd->getClassSize(argSigClass));
}
#if defined(TARGET_AMD64)
#ifdef UNIX_AMD64_ABI
if (!isStructArg)
{
size = 1; // On AMD64, all primitives fit in a single (64-bit) 'slot'
byteSize = genTypeSize(arg.GetSignatureType());
byteSize = genTypeSize(argSigType);
}
else
{
size = (unsigned)(roundUp(structSize, TARGET_POINTER_SIZE)) / TARGET_POINTER_SIZE;
byteSize = structSize;
comp->eeGetSystemVAmd64PassStructInRegisterDescriptor(objClass, &structDesc);
comp->eeGetSystemVAmd64PassStructInRegisterDescriptor(argSigClass, &structDesc);
}
#else // !UNIX_AMD64_ABI
size = 1; // On AMD64 Windows, all args fit in a single (64-bit) 'slot'
if (!isStructArg)
{
byteSize = genTypeSize(arg.GetSignatureType());
byteSize = genTypeSize(argSigType);
}

#endif // UNIX_AMD64_ABI
Expand All @@ -2402,9 +2407,8 @@ void CallArgs::AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call
{
// HFA structs are passed by value in multiple registers.
// The "size" in registers may differ the size in pointer-sized units.
CORINFO_CLASS_HANDLE structHnd = comp->gtGetStructHandle(argx);
size = comp->GetHfaCount(structHnd);
byteSize = comp->info.compCompHnd->getClassSize(structHnd);
size = hfaSlots;
byteSize = structSize;
}
else
{
Expand All @@ -2420,13 +2424,13 @@ void CallArgs::AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call
size = 1;
}
}
// Note that there are some additional rules for multireg structs.
// Note that there are some additional rules for multireg structs on ARM64.
// (i.e they cannot be split between registers and the stack)
}
else
{
size = 1; // Otherwise, all primitive types fit in a single (64-bit) 'slot'
byteSize = genTypeSize(arg.GetSignatureType());
byteSize = genTypeSize(argSigType);
}
#elif defined(TARGET_ARM) || defined(TARGET_X86)
if (isStructArg)
Expand All @@ -2438,8 +2442,8 @@ void CallArgs::AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call
{
// The typical case.
// Long/double type argument(s) will be modified as needed in Lowering.
size = genTypeStSz(argx->gtType);
byteSize = genTypeSize(arg.GetSignatureType());
size = genTypeStSz(argSigType);
byteSize = genTypeSize(argSigType);
}
#else
#error Unsupported or unset target architecture
Expand All @@ -2451,14 +2455,14 @@ void CallArgs::AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call
assert(structSize != 0);

Compiler::structPassingKind howToPassStruct;
structBaseType = comp->getArgTypeForStruct(objClass, &howToPassStruct, callIsVararg, structSize);
structBaseType = comp->getArgTypeForStruct(argSigClass, &howToPassStruct, callIsVararg, structSize);
passStructByRef = (howToPassStruct == Compiler::SPK_ByReference);
#if defined(TARGET_LOONGARCH64)
if (!passStructByRef)
{
assert((howToPassStruct == Compiler::SPK_ByValue) || (howToPassStruct == Compiler::SPK_PrimitiveType));

floatFieldFlags = comp->info.compCompHnd->getLoongArch64PassStructInRegisterFlags(objClass);
floatFieldFlags = comp->info.compCompHnd->getLoongArch64PassStructInRegisterFlags(argSigClass);

passUsingFloatRegs = (floatFieldFlags & STRUCT_HAS_FLOAT_FIELDS_MASK) ? true : false;
comp->compFloatingPointUsed |= passUsingFloatRegs;
Expand All @@ -2469,8 +2473,7 @@ void CallArgs::AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call
// for "struct { float, float }", and retyping to a primitive here will cause the
// multi-reg morphing to not kick in (the struct in question needs to be passed in
// two FP registers). Here is just keep "structBaseType" as "TYP_STRUCT".
// TODO-LoongArch64: fix "getPrimitiveTypeForStruct" or use the ABI information in
// the arg entry instead of calling it here.
// TODO-LoongArch64: fix "getPrimitiveTypeForStruct".
structBaseType = TYP_STRUCT;
}

Expand Down Expand Up @@ -2529,7 +2532,7 @@ void CallArgs::AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call
// Arm64 Apple has a special ABI for passing small size arguments on stack,
// bytes are aligned to 1-byte, shorts to 2-byte, int/float to 4-byte, etc.
// It means passing 8 1-byte arguments on stack can take as small as 8 bytes.
argAlignBytes = comp->eeGetArgSizeAlignment(arg.GetSignatureType(), isFloatHfa);
argAlignBytes = comp->eeGetArgSizeAlignment(argSigType, isFloatHfa);
}

#ifdef TARGET_LOONGARCH64
Expand All @@ -2541,11 +2544,11 @@ void CallArgs::AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call
bool isRegArg = false;
regNumber nonStdRegNum = REG_NA;

if (isRegParamType(genActualType(argx->TypeGet()))
if (isRegParamType(genActualType(argSigType))
#ifdef UNIX_AMD64_ABI
&& (!isStructArg || structDesc.passedInRegisters)
#elif defined(TARGET_X86)
|| (isStructArg && comp->isTrivialPointerSizedStruct(objClass))
|| (isStructArg && comp->isTrivialPointerSizedStruct(argSigClass))
#endif
)
{
Expand Down Expand Up @@ -3001,12 +3004,9 @@ void CallArgs::AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call
#endif
}

if (GlobalJitOptions::compFeatureHfa)
if (isHfaArg)
{
if (isHfaArg)
{
arg.AbiInfo.SetHfaType(hfaType, hfaSlots);
}
arg.AbiInfo.SetHfaType(hfaType, hfaSlots);
}

arg.AbiInfo.SetMultiRegNums();
Expand Down

0 comments on commit 6418252

Please sign in to comment.