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.

Author: SingleAccretion

src/coreclr/jit/codegencommon.cpp

@@ -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)

src/coreclr/jit/compiler.h

@@ -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);
 

src/coreclr/jit/lower.cpp

@@ -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;

src/coreclr/jit/morph.cpp

@@ -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;
@@ -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)
@@ -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
@@ -2272,10 +2277,11 @@ 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().
+        // TODO-Bug: this should use the signature type/class instead of "argx".
         comp->codeGen->InferOpSizeAlign(argx, &argAlignBytes);
 
         argAlignBytes = roundUp(argAlignBytes, TARGET_POINTER_SIZE);
@@ -2303,11 +2309,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)
 
@@ -2316,7 +2322,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
@@ -2325,43 +2331,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");
@@ -2370,28 +2367,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
@@ -2402,9 +2400,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
             {
@@ -2420,13 +2417,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)
@@ -2438,8 +2435,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
@@ -2451,14 +2448,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;
@@ -2469,8 +2466,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;
                 }
 
@@ -2529,7 +2525,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
@@ -2541,11 +2537,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
                 )
         {
@@ -3001,12 +2997,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();