From 0aa3197981bf8be2b60ee7ddf81af163d71c77d9 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Sun, 25 May 2025 19:38:10 +0200 Subject: [PATCH 1/5] JIT: Support bitwise field insertions for call arguments Add support using bitwise operations to reconstruct registers passed into calls from multiple promoted fields. Remove the IR invariant that `FIELD_LIST` args must always map cleanly to registers; instead, any `FIELD_LIST` is allowed for register-only arguments before lowering, and lowering takes care to normalize them into a handled shape. `fgTryMorphStructArg` is changed to take advantage of this by now producing `FIELD_LIST` even when a promoted arg does not match the target ABI. Support in physical promotion will be added in a follow up. Split arguments are not handled and retain the old IR invariant of requiring registers and stack slots to make cleanly from `FIELD_LIST`. win-x64 examples: ```csharp static void Foo(int x) { Use(x); Use(5); Use(null); } ``` ```diff G_M7200_IG02: ;; offset=0x0004 - mov byte ptr [rsp+0x20], 1 - mov dword ptr [rsp+0x24], ecx - mov rcx, qword ptr [rsp+0x20] + mov ecx, ecx + shl rcx, 32 + or rcx, 1 call [Program:Bar(System.Nullable`1[int])] - mov dword ptr [rsp+0x24], 5 - mov rcx, qword ptr [rsp+0x20] + mov rcx, 0x500000001 call [Program:Bar(System.Nullable`1[int])] - mov byte ptr [rsp+0x20], 0 xor ecx, ecx - mov dword ptr [rsp+0x24], ecx - mov rcx, qword ptr [rsp+0x20] - ;; size=55 bbWeight=1 PerfScore 14.25 + ;; size=34 bbWeight=1 PerfScore 7.50 G_M7200_IG03: add rsp, 40 tail.jmp [Program:Bar(System.Nullable`1[int])] ;; size=10 bbWeight=1 PerfScore 2.25 ``` ```csharp static void Foo(int x, float y) { Use((x, y)); } ``` ```diff G_M42652_IG01: ;; offset=0x0000 - push rax - ;; size=1 bbWeight=1 PerfScore 1.00 + ;; size=0 bbWeight=1 PerfScore 0.00 G_M42652_IG02: - mov dword ptr [rsp], ecx - vmovss dword ptr [rsp+0x04], xmm1 - mov rcx, qword ptr [rsp] + vmovd eax, xmm1 + shl rax, 32 + mov ecx, ecx + or rcx, rax ;; size=13 bbWeight=1 PerfScore 3.00 G_M42652_IG03: - add rsp, 8 tail.jmp [Program:Use[System.ValueTuple`2[int,float]](System.ValueTuple`2[int,float])] ``` A win-arm64 example: ```diff G_M33990_IG01: - stp fp, lr, [sp, #-0x20]! + stp fp, lr, [sp, #-0x10]! mov fp, sp - str xzr, [fp, #0x10] // [V03 tmp2] - ;; size=12 bbWeight=1 PerfScore 2.50 + ;; size=8 bbWeight=1 PerfScore 1.50 G_M33990_IG02: cbz x0, G_M33990_IG04 ;; size=4 bbWeight=1 PerfScore 1.00 G_M33990_IG03: - str x0, [fp, #0x10] // [V07 tmp6] - str wzr, [fp, #0x18] // [V08 tmp7] - ldr w0, [x0, #0x08] - str w0, [fp, #0x1C] // [V09 tmp8] + ldr w1, [x0, #0x08] b G_M33990_IG05 - ;; size=20 bbWeight=0.50 PerfScore 3.50 + ;; size=8 bbWeight=0.50 PerfScore 2.00 G_M33990_IG04: - str xzr, [fp, #0x10] // [V07 tmp6] - str xzr, [fp, #0x18] - ;; size=8 bbWeight=0.50 PerfScore 1.00 + mov x0, xzr + mov w1, wzr + ;; size=8 bbWeight=0.50 PerfScore 0.50 G_M33990_IG05: - ldp x0, x1, [fp, #0x10] // [V03 tmp2], [V03 tmp2+0x08] - movz x2, #0xD920 // code for Program:Use[System.Memory`1[int]](System.Memory`1[int]) - movk x2, #0x4590 LSL #16 + mov w1, w1 + lsl x1, x1, #32 + movz x2, #0xD950 // code for Program:Use[System.Memory`1[int]](System.Memory`1[int]) + movk x2, #0x4592 LSL #16 movk x2, #0x7FFE LSL #32 ldr x2, [x2] - ;; size=20 bbWeight=1 PerfScore 7.50 + ;; size=24 bbWeight=1 PerfScore 6.00 G_M33990_IG06: - ldp fp, lr, [sp], #0x20 + ldp fp, lr, [sp], #0x10 br x2 ;; size=8 bbWeight=1 PerfScore 2.00 -; Total bytes of code: 72 +; Total bytes of code: 60 ``` --- src/coreclr/jit/abi.cpp | 23 ++++ src/coreclr/jit/abi.h | 1 + src/coreclr/jit/codegencommon.cpp | 21 +--- src/coreclr/jit/compiler.h | 1 + src/coreclr/jit/gentree.h | 16 +-- src/coreclr/jit/lower.cpp | 168 ++++++++++++++++++++++-------- src/coreclr/jit/lower.h | 12 ++- src/coreclr/jit/morph.cpp | 127 +++++++++++----------- 8 files changed, 236 insertions(+), 133 deletions(-) diff --git a/src/coreclr/jit/abi.cpp b/src/coreclr/jit/abi.cpp index 0525e18888f46b..198f523f6b6b70 100644 --- a/src/coreclr/jit/abi.cpp +++ b/src/coreclr/jit/abi.cpp @@ -154,6 +154,29 @@ var_types ABIPassingSegment::GetRegisterType() const } } +//----------------------------------------------------------------------------- +// GetRegisterType: +// Return the smallest type larger or equal to Size that most naturally +// represents the register this segment is passed in, taking into account the +// GC info of the specified layout. +// +// Return Value: +// A type that matches ABIPassingSegment::Size and the register. +// +var_types ABIPassingSegment::GetRegisterType(ClassLayout* layout) const +{ + if (genIsValidIntReg(GetRegister())) + { + assert(Offset < layout->GetSize()); + if (((Offset % TARGET_POINTER_SIZE) == 0) && (Size == TARGET_POINTER_SIZE)) + { + return layout->GetGCPtrType(Offset / TARGET_POINTER_SIZE); + } + } + + return GetRegisterType(); +} + //----------------------------------------------------------------------------- // InRegister: // Create an ABIPassingSegment representing that a segment is passed in a diff --git a/src/coreclr/jit/abi.h b/src/coreclr/jit/abi.h index dcb7b73aacff57..dcb23cfd37470c 100644 --- a/src/coreclr/jit/abi.h +++ b/src/coreclr/jit/abi.h @@ -41,6 +41,7 @@ class ABIPassingSegment unsigned GetStackSize() const; var_types GetRegisterType() const; + var_types GetRegisterType(ClassLayout* layout) const; static ABIPassingSegment InRegister(regNumber reg, unsigned offset, unsigned size); static ABIPassingSegment OnStack(unsigned stackOffset, unsigned offset, unsigned size); diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index a56f8edcc9e391..33c04d97c9037c 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -2895,34 +2895,23 @@ var_types CodeGen::genParamStackType(LclVarDsc* dsc, const ABIPassingSegment& se return dsc->TypeGet(); case TYP_STRUCT: { - if (genIsValidFloatReg(seg.GetRegister())) - { - return seg.GetRegisterType(); - } - - ClassLayout* layout = dsc->GetLayout(); - assert(seg.Offset < layout->GetSize()); - if (((seg.Offset % TARGET_POINTER_SIZE) == 0) && (seg.Size == TARGET_POINTER_SIZE)) - { - return layout->GetGCPtrType(seg.Offset / TARGET_POINTER_SIZE); - } - + var_types type = seg.GetRegisterType(dsc->GetLayout()); // For the Swift calling convention the enregistered segments do // not match the memory layout, so we need to use exact store sizes // for the same reason as RISCV64/LA64 below. if (compiler->info.compCallConv == CorInfoCallConvExtension::Swift) { - return seg.GetRegisterType(); + return type; } #if defined(TARGET_ARM64) // We round struct sizes up to TYP_I_IMPL on the stack frame so we // can always use the full register size here. This allows us to // use stp more often. - return TYP_I_IMPL; + return genTypeSize(type) < TARGET_POINTER_SIZE ? TYP_I_IMPL : type; #elif defined(TARGET_XARCH) // Round up to use smallest possible encoding - return genActualType(seg.GetRegisterType()); + return genActualType(type); #else // On other platforms, a safer default is to use the exact size always. For example, for // RISC-V/LoongArch structs passed according to floating-point calling convention are enregistered one @@ -2930,7 +2919,7 @@ var_types CodeGen::genParamStackType(LclVarDsc* dsc, const ABIPassingSegment& se // must not be upsized to 4 bytes, otherwise for example: // * struct { struct{} e1,e2,e3; byte b; float f; } -- 4-byte store for 'b' would trash 'f' // * struct { float f; struct{} e1,e2,e3; byte b; } -- 4-byte store for 'b' would trash adjacent stack slot - return seg.GetRegisterType(); + return type; #endif } default: diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 63baaf1cc386d3..fa0fc56cd759bc 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -11633,6 +11633,7 @@ class Compiler #endif // defined(UNIX_AMD64_ABI) bool fgTryMorphStructArg(CallArg* arg); + bool FieldsMatchAbi(LclVarDsc* varDsc, const ABIPassingInformation& abiInfo); bool killGCRefs(GenTree* tree); diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index cc33d1c00b0bf7..56be3ec5750fe7 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -2725,16 +2725,10 @@ struct GenTreeFieldList : public GenTree class UseList { - Use* m_head; - Use* m_tail; + Use* m_head = nullptr; + Use* m_tail = nullptr; public: - UseList() - : m_head(nullptr) - , m_tail(nullptr) - { - } - Use* GetHead() const { return m_head; @@ -2792,6 +2786,12 @@ struct GenTreeFieldList : public GenTree } } + void Clear() + { + m_head = nullptr; + m_tail = nullptr; + } + bool IsSorted() const { unsigned offset = 0; diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 5b13b6494351ae..33ea0e950d7ade 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -1725,20 +1725,12 @@ void Lowering::LowerArg(GenTreeCall* call, CallArg* callArg) { if (abiInfo.HasAnyRegisterSegment()) { -#if FEATURE_MULTIREG_ARGS - if ((abiInfo.NumSegments > 1) && arg->OperIs(GT_FIELD_LIST)) + if (arg->OperIs(GT_FIELD_LIST)) { - unsigned int regIndex = 0; - for (GenTreeFieldList::Use& use : arg->AsFieldList()->Uses()) - { - const ABIPassingSegment& segment = abiInfo.Segment(regIndex); - InsertPutArgReg(&use.NodeRef(), segment); - - regIndex++; - } + LowerArgFieldList(callArg, arg->AsFieldList()); + arg = *ppArg; } else -#endif // FEATURE_MULTIREG_ARGS { assert(abiInfo.HasExactlyOneRegisterSegment()); InsertPutArgReg(ppArg, abiInfo.Segment(0)); @@ -4809,6 +4801,18 @@ void Lowering::LowerRet(GenTreeOp* ret) ContainCheckRet(ret); } +struct LowerFieldListRegisterInfo +{ + unsigned Offset; + var_types RegType; + + LowerFieldListRegisterInfo(unsigned offset, var_types regType) + : Offset(offset) + , RegType(regType) + { + } +}; + //---------------------------------------------------------------------------------------------- // LowerRetFieldList: // Lower a returned FIELD_LIST node. @@ -4822,21 +4826,18 @@ void Lowering::LowerRetFieldList(GenTreeOp* ret, GenTreeFieldList* fieldList) const ReturnTypeDesc& retDesc = comp->compRetTypeDesc; unsigned numRegs = retDesc.GetReturnRegCount(); - bool isCompatible = IsFieldListCompatibleWithReturn(fieldList); + auto getRegInfo = [=, &retDesc](unsigned regIndex) { + unsigned offset = retDesc.GetReturnFieldOffset(regIndex); + var_types regType = genActualType(retDesc.GetReturnRegType(regIndex)); + return LowerFieldListRegisterInfo(offset, regType); + }; + + bool isCompatible = IsFieldListCompatibleWithRegisters(fieldList, numRegs, getRegInfo); if (!isCompatible) { - JITDUMP("Spilling field list [%06u] to stack\n", Compiler::dspTreeID(fieldList)); - unsigned lclNum = comp->lvaGrabTemp(true DEBUGARG("Spilled local for return value")); + unsigned lclNum = + StoreFieldListToNewLocal(comp->typGetObjLayout(comp->info.compMethodInfo->args.retTypeClass), fieldList); LclVarDsc* varDsc = comp->lvaGetDesc(lclNum); - comp->lvaSetStruct(lclNum, comp->info.compMethodInfo->args.retTypeClass, false); - comp->lvaSetVarDoNotEnregister(lclNum DEBUGARG(DoNotEnregisterReason::BlockOpRet)); - - for (GenTreeFieldList::Use& use : fieldList->Uses()) - { - GenTree* store = comp->gtNewStoreLclFldNode(lclNum, use.GetType(), use.GetOffset(), use.GetNode()); - BlockRange().InsertAfter(use.GetNode(), store); - LowerNode(store); - } GenTree* retValue = comp->gtNewLclvNode(lclNum, varDsc->TypeGet()); ret->SetReturnValue(retValue); @@ -4859,7 +4860,89 @@ void Lowering::LowerRetFieldList(GenTreeOp* ret, GenTreeFieldList* fieldList) return; } - LowerFieldListToFieldListOfRegisters(fieldList); + LowerFieldListToFieldListOfRegisters(fieldList, numRegs, getRegInfo); +} + +//---------------------------------------------------------------------------------------------- +// StoreFieldListToNewLocal: +// Create a new local with the specified layout and store the specified +// fields of the specified FIELD_LIST into it. +// +// Arguments: +// layout - Layout of the new local +// fieldList - Fields to store to it +// +// Returns: +// Var number of new local. +// +unsigned Lowering::StoreFieldListToNewLocal(ClassLayout* layout, GenTreeFieldList* fieldList) +{ + JITDUMP("Spilling field list [%06u] to stack\n", Compiler::dspTreeID(fieldList)); + unsigned lclNum = comp->lvaGrabTemp(true DEBUGARG("Spilled local for field list")); + LclVarDsc* varDsc = comp->lvaGetDesc(lclNum); + comp->lvaSetStruct(lclNum, layout, false); + comp->lvaSetVarDoNotEnregister(lclNum DEBUGARG(DoNotEnregisterReason::LocalField)); + + for (GenTreeFieldList::Use& use : fieldList->Uses()) + { + GenTree* store = comp->gtNewStoreLclFldNode(lclNum, use.GetType(), use.GetOffset(), use.GetNode()); + BlockRange().InsertAfter(use.GetNode(), store); + LowerNode(store); + } + + return lclNum; +} + +//---------------------------------------------------------------------------------------------- +// LowerArgFieldList: +// Lower an argument FIELD_LIST node. +// +// Arguments: +// arg - The argument +// fieldList - The FIELD_LIST node +// +void Lowering::LowerArgFieldList(CallArg* arg, GenTreeFieldList* fieldList) +{ + assert(!arg->AbiInfo.HasAnyStackSegment()); + + auto getRegInfo = [=](unsigned regIndex) { + const ABIPassingSegment& seg = arg->AbiInfo.Segment(regIndex); + return LowerFieldListRegisterInfo(seg.Offset, seg.GetRegisterType()); + }; + + bool isCompatible = IsFieldListCompatibleWithRegisters(fieldList, arg->AbiInfo.NumSegments, getRegInfo); + if (!isCompatible) + { + ClassLayout* layout = comp->typGetObjLayout(arg->GetSignatureClassHandle()); + unsigned lclNum = StoreFieldListToNewLocal(layout, fieldList); + fieldList->Uses().Clear(); + for (const ABIPassingSegment& seg : arg->AbiInfo.Segments()) + { + GenTreeLclFld* fld = comp->gtNewLclFldNode(lclNum, seg.GetRegisterType(layout), seg.Offset); + fieldList->AddFieldLIR(comp, fld, seg.Offset, fld->TypeGet()); + BlockRange().InsertBefore(fieldList, fld); + } + } + else + { + LowerFieldListToFieldListOfRegisters(fieldList, arg->AbiInfo.NumSegments, getRegInfo); + } + + GenTreeFieldList::Use* field = fieldList->Uses().GetHead(); + for (const ABIPassingSegment& seg : arg->AbiInfo.Segments()) + { + assert((field != nullptr) && "Ran out of fields while inserting PUTARG_REG"); + InsertPutArgReg(&field->NodeRef(), seg); + field = field->GetNext(); + } + + assert((field == nullptr) && "Missed fields while inserting PUTARG_REG"); + + arg->NodeRef() = fieldList->SoleFieldOrThis(); + if (arg->GetNode() != fieldList) + { + BlockRange().Remove(fieldList); + } } //---------------------------------------------------------------------------------------------- @@ -4874,21 +4957,23 @@ void Lowering::LowerRetFieldList(GenTreeOp* ret, GenTreeFieldList* fieldList) // True if the fields of the FIELD_LIST are all direct insertions into the // return registers. // -bool Lowering::IsFieldListCompatibleWithReturn(GenTreeFieldList* fieldList) +template +bool Lowering::IsFieldListCompatibleWithRegisters(GenTreeFieldList* fieldList, + unsigned numRegs, + GetRegisterInfoFunc getRegInfo) { - JITDUMP("Checking if field list [%06u] is compatible with return ABI: ", Compiler::dspTreeID(fieldList)); - const ReturnTypeDesc& retDesc = comp->compRetTypeDesc; - unsigned numRetRegs = retDesc.GetReturnRegCount(); + JITDUMP("Checking if field list [%06u] is compatible with registers: ", Compiler::dspTreeID(fieldList)); GenTreeFieldList::Use* use = fieldList->Uses().GetHead(); - for (unsigned i = 0; i < numRetRegs; i++) + for (unsigned i = 0; i < numRegs; i++) { - unsigned regStart = retDesc.GetReturnFieldOffset(i); - var_types regType = retDesc.GetReturnRegType(i); - unsigned regEnd = regStart + genTypeSize(regType); + LowerFieldListRegisterInfo regInfo = getRegInfo(i); + unsigned regStart = regInfo.Offset; + var_types regType = regInfo.RegType; + unsigned regEnd = regStart + genTypeSize(regType); // TODO-CQ: Could just create a 0 for this. - if (use == nullptr) + if ((use == nullptr) || (use->GetOffset() >= regEnd)) { JITDUMP("it is not; register %u has no corresponding field\n", i); return false; @@ -4949,19 +5034,20 @@ bool Lowering::IsFieldListCompatibleWithReturn(GenTreeFieldList* fieldList) // Arguments: // fieldList - The field list // -void Lowering::LowerFieldListToFieldListOfRegisters(GenTreeFieldList* fieldList) +template +void Lowering::LowerFieldListToFieldListOfRegisters(GenTreeFieldList* fieldList, + unsigned numRegs, + GetRegisterInfoFunc getRegInfo) { - const ReturnTypeDesc& retDesc = comp->compRetTypeDesc; - unsigned numRegs = retDesc.GetReturnRegCount(); - GenTreeFieldList::Use* use = fieldList->Uses().GetHead(); assert(fieldList->Uses().IsSorted()); for (unsigned i = 0; i < numRegs; i++) { - unsigned regStart = retDesc.GetReturnFieldOffset(i); - var_types regType = genActualType(retDesc.GetReturnRegType(i)); - unsigned regEnd = regStart + genTypeSize(regType); + LowerFieldListRegisterInfo regInfo = getRegInfo(i); + unsigned regStart = regInfo.Offset; + var_types regType = regInfo.RegType; + unsigned regEnd = regStart + genTypeSize(regType); GenTreeFieldList::Use* regEntry = use; @@ -5001,7 +5087,7 @@ void Lowering::LowerFieldListToFieldListOfRegisters(GenTreeFieldList* fieldList) } // If this is a float -> int insertion, then we need the bitcast now. - if (varTypeUsesFloatReg(value) && varTypeUsesIntReg(regType)) + if (varTypeUsesFloatReg(value) && varTypeUsesIntReg(regInfo.RegType)) { assert((genTypeSize(value) == 4) || (genTypeSize(value) == 8)); var_types castType = genTypeSize(value) == 4 ? TYP_INT : TYP_LONG; diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index 019a2f2b48a4bd..75507bd6caa1a2 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -187,10 +187,14 @@ class Lowering final : public Phase GenTree* LowerAsyncContinuation(GenTree* asyncCont); void LowerReturnSuspend(GenTree* retSuspend); void LowerRetFieldList(GenTreeOp* ret, GenTreeFieldList* fieldList); - bool IsFieldListCompatibleWithReturn(GenTreeFieldList* fieldList); - void LowerFieldListToFieldListOfRegisters(GenTreeFieldList* fieldList); - void LowerCallStruct(GenTreeCall* call); - void LowerStoreSingleRegCallStruct(GenTreeBlk* store); + unsigned StoreFieldListToNewLocal(ClassLayout* layout, GenTreeFieldList* fieldList); + void LowerArgFieldList(CallArg* arg, GenTreeFieldList* fieldList); + template + bool IsFieldListCompatibleWithRegisters(GenTreeFieldList* fieldList, unsigned numRegs, GetRegisterInfoFunc func); + template + void LowerFieldListToFieldListOfRegisters(GenTreeFieldList* fieldList, unsigned numRegs, GetRegisterInfoFunc func); + void LowerCallStruct(GenTreeCall* call); + void LowerStoreSingleRegCallStruct(GenTreeBlk* store); #if !defined(WINDOWS_AMD64_ABI) GenTreeLclVar* SpillStructCallResult(GenTreeCall* call) const; #endif // WINDOWS_AMD64_ABI diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index de7fe9bfd19269..4e8002ad2161d0 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -2219,11 +2219,11 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) // False if the argument cannot be put into a shape supported by the backend. // // Remarks: -// The backend requires register-passed arguments to be of FIELD_LIST shape -// with one primitive node for each register. If the argument is passed in -// only one register, then the FIELD_LIST should be directly replaced by its -// operand. For stack-passed arguments the backend supports struct-typed -// arguments directly. +// The backend requires register-passed arguments to be of FIELD_LIST shape. +// For split arguments it is additionally required that registers and stack +// slots have clean mappings to fields. +// For stack-passed arguments the backend supports struct-typed arguments +// directly. // bool Compiler::fgTryMorphStructArg(CallArg* arg) { @@ -2283,6 +2283,13 @@ bool Compiler::fgTryMorphStructArg(CallArg* arg) return true; } + if (argNode->OperIsFieldList()) + { + // We can already see a field list here if physical promotion created + // it. + return true; + } + GenTree* newArg = nullptr; if (argNode->OperIs(GT_LCL_VAR)) @@ -2299,53 +2306,13 @@ bool Compiler::fgTryMorphStructArg(CallArg* arg) JITDUMP("Struct argument V%02u: ", lclNum); JITDUMPEXEC(arg->Dump(this)); - // Try to see if we can use the promoted fields to pass this argument. + // Try to see if we can and should use promoted fields to pass this + // argument. // - if (varDsc->lvPromoted && !varDsc->lvDoNotEnregister && - (varDsc->lvFieldCnt == arg->AbiInfo.CountRegsAndStackSlots())) + if (varDsc->lvPromoted && !varDsc->lvDoNotEnregister && (!isSplit || FieldsMatchAbi(varDsc, arg->AbiInfo))) { - bool fieldsMatch = true; - - for (const ABIPassingSegment& seg : arg->AbiInfo.Segments()) - { - if (seg.IsPassedInRegister()) - { - unsigned fieldLclNum = lvaGetFieldLocal(varDsc, seg.Offset); - if (fieldLclNum == BAD_VAR_NUM) - { - fieldsMatch = false; - break; - } - } - else - { - for (unsigned offset = 0; offset < seg.Size; offset += TARGET_POINTER_SIZE) - { - if (lvaGetFieldLocal(varDsc, seg.Offset + offset) == BAD_VAR_NUM) - { - fieldsMatch = false; - break; - } - } - } - } - - if (fieldsMatch) - { - newArg = fgMorphLclToFieldList(lclNode)->SoleFieldOrThis(); - newArg = fgMorphTree(newArg); - } - } - } - else if (argNode->OperIsFieldList()) - { - // We can already see a field list here if physical promotion created it. - // Physical promotion will also create single-field field lists which - // the backend does not expect, so fix that here. - newArg = argNode->AsFieldList()->SoleFieldOrThis(); - if (newArg == argNode) - { - return true; + newArg = fgMorphLclToFieldList(lclNode); + newArg = fgMorphTree(newArg); } } @@ -2353,20 +2320,7 @@ bool Compiler::fgTryMorphStructArg(CallArg* arg) // if (newArg == nullptr) { - bool isUseOfIndependentlyPromotedStruct = - argNode->OperIs(GT_LCL_VAR) && - (lvaGetPromotionType(argNode->AsLclVarCommon()->GetLclNum()) == PROMOTION_TYPE_INDEPENDENT); - if (isUseOfIndependentlyPromotedStruct) - { - if (arg->AbiInfo.HasExactlyOneRegisterSegment()) - { - // We already tried to use the fields above, but that failed. - // Here we prefer to create a copy to avoid DNER'ing the local. - // That turns out to be profitable mostly for single-register arguments. - return false; - } - } - else if (!argNode->TypeIs(TYP_STRUCT) && arg->AbiInfo.HasExactlyOneRegisterSegment()) + if (!argNode->TypeIs(TYP_STRUCT) && arg->AbiInfo.HasExactlyOneRegisterSegment()) { // This can be treated primitively. Leave it alone. return true; @@ -2544,6 +2498,51 @@ bool Compiler::fgTryMorphStructArg(CallArg* arg) return true; } +//----------------------------------------------------------------------------- +// FieldsMatchAbi: +// Check if the fields of a local map cleanly (in terms of offsets) to the +// specified ABI info. +// +// Arguments: +// varDsc - promoted local +// abiInfo - ABI information +// +// Returns: +// True if it does. In that case FIELD_LIST usage is allowed for split args +// by the backend. +// +bool Compiler::FieldsMatchAbi(LclVarDsc* varDsc, const ABIPassingInformation& abiInfo) +{ + if (varDsc->lvFieldCnt != abiInfo.CountRegsAndStackSlots()) + { + return false; + } + + for (const ABIPassingSegment& seg : abiInfo.Segments()) + { + if (seg.IsPassedInRegister()) + { + unsigned fieldLclNum = lvaGetFieldLocal(varDsc, seg.Offset); + if (fieldLclNum == BAD_VAR_NUM) + { + return false; + } + } + else + { + for (unsigned offset = 0; offset < seg.Size; offset += TARGET_POINTER_SIZE) + { + if (lvaGetFieldLocal(varDsc, seg.Offset + offset) == BAD_VAR_NUM) + { + return false; + } + } + } + } + + return true; +} + //------------------------------------------------------------------------ // fgMorphLclToFieldList: Morph a GT_LCL_VAR node to a GT_FIELD_LIST of its promoted fields // From 3b2b47b9bae5e27901aa682cedbdb99b2b76511b Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 26 May 2025 16:03:34 +0200 Subject: [PATCH 2/5] Revert `genParamStackType` change Wrong for floats... --- src/coreclr/jit/codegencommon.cpp | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 33c04d97c9037c..a56f8edcc9e391 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -2895,23 +2895,34 @@ var_types CodeGen::genParamStackType(LclVarDsc* dsc, const ABIPassingSegment& se return dsc->TypeGet(); case TYP_STRUCT: { - var_types type = seg.GetRegisterType(dsc->GetLayout()); + if (genIsValidFloatReg(seg.GetRegister())) + { + return seg.GetRegisterType(); + } + + ClassLayout* layout = dsc->GetLayout(); + assert(seg.Offset < layout->GetSize()); + if (((seg.Offset % TARGET_POINTER_SIZE) == 0) && (seg.Size == TARGET_POINTER_SIZE)) + { + return layout->GetGCPtrType(seg.Offset / TARGET_POINTER_SIZE); + } + // For the Swift calling convention the enregistered segments do // not match the memory layout, so we need to use exact store sizes // for the same reason as RISCV64/LA64 below. if (compiler->info.compCallConv == CorInfoCallConvExtension::Swift) { - return type; + return seg.GetRegisterType(); } #if defined(TARGET_ARM64) // We round struct sizes up to TYP_I_IMPL on the stack frame so we // can always use the full register size here. This allows us to // use stp more often. - return genTypeSize(type) < TARGET_POINTER_SIZE ? TYP_I_IMPL : type; + return TYP_I_IMPL; #elif defined(TARGET_XARCH) // Round up to use smallest possible encoding - return genActualType(type); + return genActualType(seg.GetRegisterType()); #else // On other platforms, a safer default is to use the exact size always. For example, for // RISC-V/LoongArch structs passed according to floating-point calling convention are enregistered one @@ -2919,7 +2930,7 @@ var_types CodeGen::genParamStackType(LclVarDsc* dsc, const ABIPassingSegment& se // must not be upsized to 4 bytes, otherwise for example: // * struct { struct{} e1,e2,e3; byte b; float f; } -- 4-byte store for 'b' would trash 'f' // * struct { float f; struct{} e1,e2,e3; byte b; } -- 4-byte store for 'b' would trash adjacent stack slot - return type; + return seg.GetRegisterType(); #endif } default: From ea33fd60456a131d4fe900e4025dfe81e73d4277 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 27 May 2025 16:10:41 +0200 Subject: [PATCH 3/5] Fix some regressions --- src/coreclr/jit/lower.cpp | 2 +- src/coreclr/jit/morph.cpp | 20 ++++++++++++-------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 33ea0e950d7ade..24e6e6f84849e4 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -4970,7 +4970,7 @@ bool Lowering::IsFieldListCompatibleWithRegisters(GenTreeFieldList* fieldList, LowerFieldListRegisterInfo regInfo = getRegInfo(i); unsigned regStart = regInfo.Offset; var_types regType = regInfo.RegType; - unsigned regEnd = regStart + genTypeSize(regType); + unsigned regEnd = regStart + (varTypeUsesFloatReg(regType) ? genTypeSize(regType) : REGSIZE_BYTES); // TODO-CQ: Could just create a 0 for this. if ((use == nullptr) || (use->GetOffset() >= regEnd)) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 4e8002ad2161d0..7ed36b5f6fc246 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -2283,13 +2283,6 @@ bool Compiler::fgTryMorphStructArg(CallArg* arg) return true; } - if (argNode->OperIsFieldList()) - { - // We can already see a field list here if physical promotion created - // it. - return true; - } - GenTree* newArg = nullptr; if (argNode->OperIs(GT_LCL_VAR)) @@ -2311,10 +2304,21 @@ bool Compiler::fgTryMorphStructArg(CallArg* arg) // if (varDsc->lvPromoted && !varDsc->lvDoNotEnregister && (!isSplit || FieldsMatchAbi(varDsc, arg->AbiInfo))) { - newArg = fgMorphLclToFieldList(lclNode); + newArg = fgMorphLclToFieldList(lclNode)->SoleFieldOrThis(); newArg = fgMorphTree(newArg); } } + else if (argNode->OperIsFieldList()) + { + // We can already see a field list here if physical promotion created it. + // Physical promotion will also create single-field field lists which + // not everything treats the same as a single node, so fix that here. + newArg = argNode->AsFieldList()->SoleFieldOrThis(); + if (newArg == argNode) + { + return true; + } + } // If we were not able to use the promoted fields... // From bd519303e9ad7d64c8ac4bb51f9f0ce1b3933aec Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 27 May 2025 16:18:45 +0200 Subject: [PATCH 4/5] Only for last register --- src/coreclr/jit/lower.cpp | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 24e6e6f84849e4..f4f11c661fe140 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -4970,7 +4970,13 @@ bool Lowering::IsFieldListCompatibleWithRegisters(GenTreeFieldList* fieldList, LowerFieldListRegisterInfo regInfo = getRegInfo(i); unsigned regStart = regInfo.Offset; var_types regType = regInfo.RegType; - unsigned regEnd = regStart + (varTypeUsesFloatReg(regType) ? genTypeSize(regType) : REGSIZE_BYTES); + unsigned regEnd = regStart + genTypeSize(regType); + + if ((i == numRegs - 1) && !varTypeUsesFloatReg(regType)) + { + // Allow tail end to pass undefined bits into the register + regEnd = regStart + REGSIZE_BYTES; + } // TODO-CQ: Could just create a 0 for this. if ((use == nullptr) || (use->GetOffset() >= regEnd)) @@ -5049,6 +5055,12 @@ void Lowering::LowerFieldListToFieldListOfRegisters(GenTreeFieldList* fieldLis var_types regType = regInfo.RegType; unsigned regEnd = regStart + genTypeSize(regType); + if ((i == numRegs - 1) && !varTypeUsesFloatReg(regType)) + { + // Allow tail end to pass undefined bits into the register + regEnd = regStart + REGSIZE_BYTES; + } + GenTreeFieldList::Use* regEntry = use; assert(use != nullptr); From f55ba17357b23f541ddbada47892898648b8f05c Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 27 May 2025 18:16:15 +0200 Subject: [PATCH 5/5] Handle primitives passed in multiple registers --- src/coreclr/jit/lower.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index f4f11c661fe140..f961a27d8585e3 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -1725,8 +1725,18 @@ void Lowering::LowerArg(GenTreeCall* call, CallArg* callArg) { if (abiInfo.HasAnyRegisterSegment()) { - if (arg->OperIs(GT_FIELD_LIST)) + if (arg->OperIs(GT_FIELD_LIST) || (abiInfo.NumSegments > 1)) { + if (!arg->OperIs(GT_FIELD_LIST)) + { + // Primitive arg, but the ABI requires it to be split into + // registers. Insert the field list here. + GenTreeFieldList* fieldList = comp->gtNewFieldList(); + fieldList->AddFieldLIR(comp, arg, 0, genActualType(arg->TypeGet())); + BlockRange().InsertAfter(arg, fieldList); + arg = *ppArg = fieldList; + } + LowerArgFieldList(callArg, arg->AsFieldList()); arg = *ppArg; }