From 25ab5699d8b3641bb3cbc52dc1d115bdb72a3230 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Fri, 19 Apr 2024 08:43:16 +0200 Subject: [PATCH 01/14] Code review from #101114 --- src/coreclr/jit/abi.h | 4 ++-- src/coreclr/jit/targetriscv64.cpp | 24 +++++++++--------------- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/src/coreclr/jit/abi.h b/src/coreclr/jit/abi.h index ac0ad5090dcf2b..37268ce3effd9f 100644 --- a/src/coreclr/jit/abi.h +++ b/src/coreclr/jit/abi.h @@ -51,8 +51,8 @@ struct ABIPassingInformation // - On loongarch64/riscv64, structs can be passed in two registers or // can be split out over register and stack, giving // multiple register segments and a struct segment. - unsigned NumSegments = 0; - ABIPassingSegment* Segments = nullptr; + unsigned NumSegments; + ABIPassingSegment* Segments; ABIPassingInformation(unsigned numSegments = 0, ABIPassingSegment* segments = nullptr) : NumSegments(numSegments) diff --git a/src/coreclr/jit/targetriscv64.cpp b/src/coreclr/jit/targetriscv64.cpp index 29f71dc76d8fa2..f7981e408032d7 100644 --- a/src/coreclr/jit/targetriscv64.cpp +++ b/src/coreclr/jit/targetriscv64.cpp @@ -36,6 +36,7 @@ RiscV64Classifier::RiscV64Classifier(const ClassifierInfo& info) , m_intRegs(intArgRegs, ArrLen(intArgRegs)) , m_floatRegs(fltArgRegs, ArrLen(fltArgRegs)) { + assert(!m_info.IsVarArgs); // TODO: varargs currently not supported on RISC-V } //----------------------------------------------------------------------------- @@ -57,8 +58,6 @@ ABIPassingInformation RiscV64Classifier::Classify(Compiler* comp, ClassLayout* structLayout, WellKnownArg /*wellKnownParam*/) { - assert(!m_info.IsVarArgs); // TODO: varargs currently not supported on RISC-V - StructFloatFieldInfoFlags flags = STRUCT_NO_FLOAT_FIELD; unsigned intFields = 0, floatFields = 0; unsigned passedSize; @@ -93,17 +92,14 @@ ABIPassingInformation RiscV64Classifier::Classify(Compiler* comp, } else { - assert(genTypeSize(type) <= TARGET_POINTER_SIZE); - - if (varTypeIsFloating(type)) - floatFields = 1; - passedSize = genTypeSize(type); + assert(passedSize <= TARGET_POINTER_SIZE); + floatFields = varTypeIsFloating(type) ? 1 : 0; } assert((floatFields > 0) || (intFields == 0)); - auto PassSlot = [this](bool inFloatReg, unsigned offset, unsigned size) -> ABIPassingSegment { + auto passSlot = [this](bool inFloatReg, unsigned offset, unsigned size) -> ABIPassingSegment { assert(size > 0); assert(size <= TARGET_POINTER_SIZE); if (inFloatReg) @@ -151,8 +147,8 @@ ABIPassingInformation RiscV64Classifier::Classify(Compiler* comp, bool isSecondFloat = (flags & (STRUCT_FLOAT_FIELD_ONLY_TWO | STRUCT_FLOAT_FIELD_SECOND)) != 0; assert(isFirstFloat || isSecondFloat); - return {2, new (comp, CMK_ABI) ABIPassingSegment[]{PassSlot(isFirstFloat, 0, firstSize), - PassSlot(isSecondFloat, offset, secondSize)}}; + return {2, new (comp, CMK_ABI) ABIPassingSegment[]{passSlot(isFirstFloat, 0, firstSize), + passSlot(isSecondFloat, offset, secondSize)}}; } } else @@ -160,18 +156,16 @@ ABIPassingInformation RiscV64Classifier::Classify(Compiler* comp, // Integer calling convention if (passedSize <= TARGET_POINTER_SIZE) { - return ABIPassingInformation::FromSegment(comp, PassSlot(false, 0, passedSize)); + return ABIPassingInformation::FromSegment(comp, passSlot(false, 0, passedSize)); } else { assert(varTypeIsStruct(type)); return {2, new (comp, CMK_ABI) - ABIPassingSegment[]{PassSlot(false, 0, TARGET_POINTER_SIZE), - PassSlot(false, TARGET_POINTER_SIZE, passedSize - TARGET_POINTER_SIZE)}}; + ABIPassingSegment[]{passSlot(false, 0, TARGET_POINTER_SIZE), + passSlot(false, TARGET_POINTER_SIZE, passedSize - TARGET_POINTER_SIZE)}}; } } - - unreached(); } #endif // TARGET_RISCV64 From 6c01d95681012dcdb7b564788e3168ecb3dfc723 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Fri, 19 Apr 2024 10:42:53 +0200 Subject: [PATCH 02/14] Use common genHomeRegisterParams on RISC-V --- src/coreclr/jit/codegencommon.cpp | 3 - src/coreclr/jit/codegenriscv64.cpp | 443 ----------------------------- 2 files changed, 446 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 417a7e6f31695d..e446e638cfabb9 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -2802,7 +2802,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX */ -#if !defined(TARGET_RISCV64) struct RegNode; struct RegNodeEdge @@ -3346,8 +3345,6 @@ void CodeGen::genHomeRegisterParams(regNumber initReg, bool* initRegStillZeroed) } } -#endif - // ----------------------------------------------------------------------------- // genGetParameterHomingTempRegisterCandidates: Get the registers that are // usable during register homing. diff --git a/src/coreclr/jit/codegenriscv64.cpp b/src/coreclr/jit/codegenriscv64.cpp index 546ba7b3180899..52749fcf087293 100644 --- a/src/coreclr/jit/codegenriscv64.cpp +++ b/src/coreclr/jit/codegenriscv64.cpp @@ -8063,449 +8063,6 @@ void CodeGen::genPopCalleeSavedRegisters(bool jmpEpilog) } } -void CodeGen::genHomeRegisterParams(regNumber initReg, bool* initRegStillZeroed) -{ - *initRegStillZeroed = false; - assert(!(intRegState.rsCalleeRegArgMaskLiveIn & floatRegState.rsCalleeRegArgMaskLiveIn)); - - regMaskTP regArgMaskLive = intRegState.rsCalleeRegArgMaskLiveIn | floatRegState.rsCalleeRegArgMaskLiveIn; - -#ifdef DEBUG - if (verbose) - { - printf("*************** In genFnPrologCalleeRegArgs() RISCV64:0x%llx.\n", regArgMaskLive); - } -#endif - - // We should be generating the prolog block when we are called - assert(compiler->compGeneratingProlog); - - // We expect to have some registers of the type we are doing, that are LiveIn, otherwise we don't need to be called. - noway_assert(regArgMaskLive != 0); - - unsigned varNum; - unsigned regArgNum = 0; - // Process any rearrangements including circular dependencies. - regNumber regArg[MAX_REG_ARG + MAX_FLOAT_REG_ARG]; - regNumber regArgInit[MAX_REG_ARG + MAX_FLOAT_REG_ARG]; - emitAttr regArgAttr[MAX_REG_ARG + MAX_FLOAT_REG_ARG]; - - for (int i = 0; i < MAX_REG_ARG + MAX_FLOAT_REG_ARG; i++) - { - regArg[i] = REG_NA; - -#ifdef DEBUG - regArgInit[i] = REG_NA; - regArgAttr[i] = EA_UNKNOWN; -#endif - } - - for (varNum = 0; varNum < compiler->lvaCount; ++varNum) - { - LclVarDsc* varDsc = compiler->lvaTable + varNum; - - // Is this variable a register arg? - if (!varDsc->lvIsParam) - { - continue; - } - - if (!varDsc->lvIsRegArg) - { - continue; - } - - if (varDsc->lvIsInReg()) - { - assert(genIsValidIntReg(varDsc->GetArgReg()) || genIsValidFloatReg(varDsc->GetArgReg())); - assert(!(genIsValidIntReg(varDsc->GetOtherArgReg()) || genIsValidFloatReg(varDsc->GetOtherArgReg()))); - if (varDsc->GetArgInitReg() != varDsc->GetArgReg()) - { - if (genIsValidIntReg(varDsc->GetArgInitReg())) - { - if (varDsc->GetArgInitReg() > REG_ARG_LAST) - { - bool isSkip; - instruction ins; - emitAttr size; - if (genIsValidIntReg(varDsc->GetArgReg())) - { - ins = INS_mov; - if (varDsc->TypeGet() == TYP_INT) - { - size = EA_4BYTE; - isSkip = false; - } - else - { - size = EA_PTRSIZE; - isSkip = true; - } - } - else - { - ins = INS_fmv_x_d; - size = EA_PTRSIZE; - isSkip = true; - } - GetEmitter()->emitIns_Mov(ins, size, varDsc->GetArgInitReg(), varDsc->GetArgReg(), isSkip); - regArgMaskLive &= ~genRegMask(varDsc->GetArgReg()); - } - else - { - if (genIsValidIntReg(varDsc->GetArgReg())) - { - assert(isValidIntArgReg(varDsc->GetArgReg(), compiler->info.compCallConv)); - regArg[varDsc->GetArgReg() - REG_ARG_FIRST] = varDsc->GetArgReg(); - regArgInit[varDsc->GetArgReg() - REG_ARG_FIRST] = varDsc->GetArgInitReg(); - regArgAttr[varDsc->GetArgReg() - REG_ARG_FIRST] = - varDsc->TypeGet() == TYP_INT ? EA_4BYTE : EA_PTRSIZE; - } - else - { - assert(isValidFloatArgReg(varDsc->GetArgReg())); - regArg[varDsc->GetArgReg() - REG_ARG_FP_FIRST + MAX_REG_ARG] = varDsc->GetArgReg(); - regArgInit[varDsc->GetArgReg() - REG_ARG_FP_FIRST + MAX_REG_ARG] = varDsc->GetArgInitReg(); - regArgAttr[varDsc->GetArgReg() - REG_ARG_FP_FIRST + MAX_REG_ARG] = - varDsc->TypeGet() == TYP_FLOAT ? EA_4BYTE : EA_PTRSIZE; - } - regArgNum++; - } - } - else - { - assert(genIsValidFloatReg(varDsc->GetArgInitReg())); - if (genIsValidIntReg(varDsc->GetArgReg())) - { - emitAttr size = (varDsc->TypeGet() == TYP_FLOAT) ? EA_4BYTE : EA_PTRSIZE; - GetEmitter()->emitIns_Mov(size, varDsc->GetArgInitReg(), varDsc->GetArgReg(), false); - regArgMaskLive &= ~genRegMask(varDsc->GetArgReg()); - } - else if (varDsc->GetArgInitReg() > REG_ARG_FP_LAST) - { - GetEmitter()->emitIns_Mov(INS_fsgnj_d, EA_PTRSIZE, varDsc->GetArgInitReg(), varDsc->GetArgReg(), - true); - regArgMaskLive &= ~genRegMask(varDsc->GetArgReg()); - } - else - { - assert(isValidFloatArgReg(varDsc->GetArgReg())); - regArg[varDsc->GetArgReg() - REG_ARG_FP_FIRST + MAX_REG_ARG] = varDsc->GetArgReg(); - regArgInit[varDsc->GetArgReg() - REG_ARG_FP_FIRST + MAX_REG_ARG] = varDsc->GetArgInitReg(); - regArgAttr[varDsc->GetArgReg() - REG_ARG_FP_FIRST + MAX_REG_ARG] = - varDsc->TypeGet() == TYP_FLOAT ? EA_4BYTE : EA_PTRSIZE; - regArgNum++; - } - } - } - else - { - // TODO-RISCV64: should delete this by optimization "struct {long a; int32_t b;};" - // liking AMD64_ABI within morph. - if (genIsValidIntReg(varDsc->GetArgReg()) && (varDsc->TypeGet() == TYP_INT)) - { - GetEmitter()->emitIns_Mov(INS_mov, EA_4BYTE, varDsc->GetArgInitReg(), varDsc->GetArgReg(), false); - } - regArgMaskLive &= ~genRegMask(varDsc->GetArgReg()); - } -#ifdef USING_SCOPE_INFO - psiMoveToReg(varNum); -#endif // USING_SCOPE_INFO - if (!varDsc->lvLiveInOutOfHndlr) - { - continue; - } - } - - // When we have a promoted struct we have two possible LclVars that can represent the incoming argument - // in the regArgTab[], either the original TYP_STRUCT argument or the introduced lvStructField. - // We will use the lvStructField if we have a TYPE_INDEPENDENT promoted struct field otherwise - // use the original TYP_STRUCT argument. - // - if (varDsc->lvPromoted || varDsc->lvIsStructField) - { - LclVarDsc* parentVarDsc = varDsc; - if (varDsc->lvIsStructField) - { - assert(!varDsc->lvPromoted); - parentVarDsc = &compiler->lvaTable[varDsc->lvParentLcl]; - } - - Compiler::lvaPromotionType promotionType = compiler->lvaGetPromotionType(parentVarDsc); - - if (promotionType == Compiler::PROMOTION_TYPE_INDEPENDENT) - { - // For register arguments that are independent promoted structs we put the promoted field varNum - // in the regArgTab[] - if (varDsc->lvPromoted) - { - continue; - } - } - else - { - // For register arguments that are not independent promoted structs we put the parent struct varNum - // in the regArgTab[] - if (varDsc->lvIsStructField) - { - continue; - } - } - } - - var_types storeType = TYP_UNDEF; - int slotSize = TARGET_POINTER_SIZE; - - if (varTypeIsStruct(varDsc)) - { - if (emitter::isFloatReg(varDsc->GetArgReg())) - { - storeType = varDsc->lvIs4Field1 ? TYP_FLOAT : TYP_DOUBLE; - } - else - { - assert(emitter::isGeneralRegister(varDsc->GetArgReg())); - if (varDsc->lvIs4Field1) - { - storeType = TYP_INT; - } - else - { - storeType = varDsc->GetLayout()->GetGCPtrType(0); - } - } - slotSize = (int)EA_SIZE(emitActualTypeSize(storeType)); - -#if FEATURE_MULTIREG_ARGS - // Must be <= MAX_PASS_MULTIREG_BYTES or else it wouldn't be passed in registers - noway_assert(varDsc->lvSize() <= MAX_PASS_MULTIREG_BYTES); -#endif - } - else // Not a struct type - { - storeType = compiler->mangleVarArgsType(genActualType(varDsc->TypeGet())); - if (emitter::isFloatReg(varDsc->GetArgReg()) != varTypeIsFloating(storeType)) - { - assert(varTypeIsFloating(storeType)); - storeType = storeType == TYP_DOUBLE ? TYP_I_IMPL : TYP_INT; - } - } - emitAttr size = emitActualTypeSize(storeType); - - regNumber srcRegNum = varDsc->GetArgReg(); - - // Stack argument - if the ref count is 0 don't care about it - if (!varDsc->lvOnFrame) - { - noway_assert(varDsc->lvRefCnt() == 0); - regArgMaskLive &= ~genRegMask(varDsc->GetArgReg()); - if (varDsc->GetOtherArgReg() < REG_STK) - { - regArgMaskLive &= ~genRegMask(varDsc->GetOtherArgReg()); - } - } - else - { - assert(srcRegNum != varDsc->GetOtherArgReg()); - - regNumber tmpReg = REG_NA; - - bool FPbased; - int baseOffset = compiler->lvaFrameAddress(varNum, &FPbased); - - // First store the `varDsc->GetArgReg()` on stack. - if (emitter::isValidSimm12(baseOffset)) - { - GetEmitter()->emitIns_S_R(ins_Store(storeType), size, srcRegNum, varNum, 0); - } - else - { - assert(tmpReg == REG_NA); - - tmpReg = REG_RA; - // Prepare tmpReg to possible future use - GetEmitter()->emitLoadImmediate(EA_PTRSIZE, tmpReg, baseOffset); - GetEmitter()->emitIns_R_R_R(INS_add, EA_PTRSIZE, tmpReg, tmpReg, FPbased ? REG_FPBASE : REG_SPBASE); - GetEmitter()->emitIns_S_R_R(ins_Store(storeType), size, srcRegNum, tmpReg, varNum, 0); - } - - regArgMaskLive &= ~genRegMask(srcRegNum); - - // Then check if varDsc is a struct arg - if (varTypeIsStruct(varDsc)) - { - if (emitter::isFloatReg(varDsc->GetOtherArgReg())) - { - srcRegNum = varDsc->GetOtherArgReg(); - storeType = varDsc->lvIs4Field2 ? TYP_FLOAT : TYP_DOUBLE; - size = EA_SIZE(emitActualTypeSize(storeType)); - - slotSize = slotSize < (int)size ? (int)size : slotSize; - } - else if (emitter::isGeneralRegister(varDsc->GetOtherArgReg())) - { - if (varDsc->lvIs4Field2) - { - storeType = TYP_INT; - } - else - { - storeType = varDsc->GetLayout()->GetGCPtrType(1); - } - - srcRegNum = varDsc->GetOtherArgReg(); - size = emitActualTypeSize(storeType); - - slotSize = slotSize < (int)EA_SIZE(size) ? (int)EA_SIZE(size) : slotSize; - } - baseOffset += slotSize; - - // if the struct passed by two register, then store the second register `varDsc->GetOtherArgReg()`. - if (srcRegNum == varDsc->GetOtherArgReg()) - { - GetEmitter()->emitIns_S_R_R(ins_Store(storeType), size, srcRegNum, tmpReg, varNum, slotSize); - regArgMaskLive &= ~genRegMask(srcRegNum); // maybe do this later is better! - } - else if (varDsc->lvIsSplit) - { - // the struct is a split struct. - assert(varDsc->GetArgReg() == REG_ARG_LAST && varDsc->GetOtherArgReg() == REG_STK); - - // For the RISCV64's ABI, the split struct arg will be passed via `A7` and a stack slot on caller. - // But if the `A7` is stored on stack on the callee side, the whole split struct should be - // stored continuous on the stack on the callee side. - // So, after we save `A7` on the stack in prolog, it has to copy the stack slot of the split struct - // which was passed by the caller. Here we load the stack slot to `REG_SCRATCH`, and save it - // on the stack following the `A7` in prolog. - if (emitter::isValidSimm12(genTotalFrameSize())) - { - GetEmitter()->emitIns_R_R_I(INS_ld, size, REG_SCRATCH, REG_SPBASE, genTotalFrameSize()); - } - else - { - assert(!EA_IS_RELOC(size)); - GetEmitter()->emitLoadImmediate(size, REG_SCRATCH, genTotalFrameSize()); - GetEmitter()->emitIns_R_R_R(INS_add, size, REG_SCRATCH, REG_SCRATCH, REG_SPBASE); - GetEmitter()->emitIns_R_R_I(INS_ld, size, REG_SCRATCH, REG_SCRATCH, 0); - } - - GetEmitter()->emitIns_S_R_R(ins_Store(storeType), size, REG_SCRATCH, tmpReg, varNum, slotSize); - } - } - -#ifdef USING_SCOPE_INFO - { - psiMoveToStack(varNum); - } -#endif // USING_SCOPE_INFO - } - } - - if (regArgNum > 0) - { - for (int i = MAX_REG_ARG + MAX_FLOAT_REG_ARG - 1; i >= 0; i--) - { - if (regArg[i] != REG_NA && !isValidIntArgReg(regArgInit[i], compiler->info.compCallConv) && - !isValidFloatArgReg(regArgInit[i])) - { - assert(regArg[i] != regArgInit[i]); - assert(isValidIntArgReg(regArg[i], compiler->info.compCallConv) || isValidFloatArgReg(regArg[i])); - - GetEmitter()->emitIns_Mov(regArgAttr[i], regArgInit[i], regArg[i], false); - - regArgMaskLive &= ~genRegMask(regArg[i]); - regArg[i] = REG_NA; - regArgNum -= 1; - } - } - } - - if (regArgNum > 0) - { - for (int i = MAX_REG_ARG + MAX_FLOAT_REG_ARG - 1; i >= 0; i--) - { - if (regArg[i] != REG_NA) - { - assert(regArg[i] != regArgInit[i]); - - // regArg indexes list - unsigned indexList[MAX_REG_ARG + MAX_FLOAT_REG_ARG]; - int count = 0; // Number of nodes in list - bool loop = false; // List has a loop - - for (unsigned cur = i; regArg[cur] != REG_NA; count++) - { - if (cur == i && count > 0) - { - loop = true; - break; - } - - indexList[count] = cur; - - for (int count2 = 0; count2 < count; count2++) - { - // The list could not have backlinks except last to first case which handled above. - assert(cur != indexList[count2] && "Attempt to move several values on same register."); - } - assert(cur < MAX_REG_ARG + MAX_FLOAT_REG_ARG); - assert(isValidIntArgReg(regArg[cur], compiler->info.compCallConv) || - isValidFloatArgReg(regArg[cur])); - - if (isValidIntArgReg(regArgInit[cur], compiler->info.compCallConv)) - { - cur = regArgInit[cur] - REG_ARG_FIRST; - } - else if (isValidFloatArgReg(regArgInit[cur])) - { - cur = regArgInit[cur] - REG_ARG_FP_FIRST + MAX_REG_ARG; - } - else - { - assert(!"Argument register is neither valid float nor valid int argument register"); - } - } - - if (loop) - { - unsigned tmpArg = indexList[count - 1]; - - GetEmitter()->emitIns_Mov(regArgAttr[tmpArg], rsGetRsvdReg(), regArg[tmpArg], false); - count--; // Decrease count to not access last node which regArgInit points to start node i - assert(count > 0); - } - - for (int cur = count - 1; cur >= 0; cur--) - { - unsigned tmpArg = indexList[cur]; - - GetEmitter()->emitIns_Mov(regArgAttr[tmpArg], regArgInit[tmpArg], regArg[tmpArg], false); - - regArgMaskLive &= ~genRegMask(regArg[tmpArg]); - regArg[tmpArg] = REG_NA; - regArgNum--; - assert(regArgNum >= 0); - }; - - if (loop) - { - unsigned tmpArg = indexList[count]; // count was decreased for loop case - - GetEmitter()->emitIns_Mov(regArgAttr[i], regArg[tmpArg], rsGetRsvdReg(), false); - - regArgMaskLive &= ~genRegMask(regArg[tmpArg]); - regArg[tmpArg] = REG_NA; - regArgNum--; - } - assert(regArgNum >= 0); - } - } - } - - assert(regArgNum == 0); - assert(!regArgMaskLive); -} - #ifdef PROFILING_SUPPORTED //----------------------------------------------------------------------------------- // genProfilingEnterCallback: Generate the profiling function enter callback. From 5423299b835cdf79ed2f316230695383962525a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Mon, 22 Apr 2024 12:14:06 +0200 Subject: [PATCH 03/14] Make passSlot integer-only because we know hardware floating-point calling convention passes in registers only --- src/coreclr/jit/targetriscv64.cpp | 50 +++++++++++++++---------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/src/coreclr/jit/targetriscv64.cpp b/src/coreclr/jit/targetriscv64.cpp index f7981e408032d7..581087b5cc63f8 100644 --- a/src/coreclr/jit/targetriscv64.cpp +++ b/src/coreclr/jit/targetriscv64.cpp @@ -99,26 +99,6 @@ ABIPassingInformation RiscV64Classifier::Classify(Compiler* comp, assert((floatFields > 0) || (intFields == 0)); - auto passSlot = [this](bool inFloatReg, unsigned offset, unsigned size) -> ABIPassingSegment { - assert(size > 0); - assert(size <= TARGET_POINTER_SIZE); - if (inFloatReg) - { - return ABIPassingSegment::InRegister(m_floatRegs.Dequeue(), offset, size); - } - else if (m_intRegs.Count() > 0) - { - return ABIPassingSegment::InRegister(m_intRegs.Dequeue(), offset, size); - } - else - { - assert((m_stackArgSize % TARGET_POINTER_SIZE) == 0); - ABIPassingSegment seg = ABIPassingSegment::OnStack(m_stackArgSize, offset, size); - m_stackArgSize += TARGET_POINTER_SIZE; - return seg; - } - }; - if ((floatFields > 0) && (m_floatRegs.Count() >= floatFields) && (m_intRegs.Count() >= intFields)) { // Hardware floating-point calling convention @@ -147,23 +127,43 @@ ABIPassingInformation RiscV64Classifier::Classify(Compiler* comp, bool isSecondFloat = (flags & (STRUCT_FLOAT_FIELD_ONLY_TWO | STRUCT_FLOAT_FIELD_SECOND)) != 0; assert(isFirstFloat || isSecondFloat); - return {2, new (comp, CMK_ABI) ABIPassingSegment[]{passSlot(isFirstFloat, 0, firstSize), - passSlot(isSecondFloat, offset, secondSize)}}; + regNumber firstReg = (isFirstFloat ? m_floatRegs : m_intRegs).Dequeue(); + regNumber secondReg = (isSecondFloat ? m_floatRegs : m_intRegs).Dequeue(); + + return {2, new (comp, CMK_ABI) + ABIPassingSegment[]{ABIPassingSegment::InRegister(firstReg, 0, firstSize), + ABIPassingSegment::InRegister(secondReg, offset, secondSize)}}; } } else { // Integer calling convention + auto passSlot = [this](unsigned offset, unsigned size) -> ABIPassingSegment { + assert(size > 0); + assert(size <= TARGET_POINTER_SIZE); + if (m_intRegs.Count() > 0) + { + return ABIPassingSegment::InRegister(m_intRegs.Dequeue(), offset, size); + } + else + { + assert((m_stackArgSize % TARGET_POINTER_SIZE) == 0); + ABIPassingSegment seg = ABIPassingSegment::OnStack(m_stackArgSize, offset, size); + m_stackArgSize += TARGET_POINTER_SIZE; + return seg; + } + }; + if (passedSize <= TARGET_POINTER_SIZE) { - return ABIPassingInformation::FromSegment(comp, passSlot(false, 0, passedSize)); + return ABIPassingInformation::FromSegment(comp, passSlot(0, passedSize)); } else { assert(varTypeIsStruct(type)); return {2, new (comp, CMK_ABI) - ABIPassingSegment[]{passSlot(false, 0, TARGET_POINTER_SIZE), - passSlot(false, TARGET_POINTER_SIZE, passedSize - TARGET_POINTER_SIZE)}}; + ABIPassingSegment[]{passSlot(0, TARGET_POINTER_SIZE), + passSlot(TARGET_POINTER_SIZE, passedSize - TARGET_POINTER_SIZE)}}; } } } From 06e9d6548130c0918a0301bd662c42152ae765f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Mon, 22 Apr 2024 09:32:54 +0200 Subject: [PATCH 04/14] Repurpose genHomeSwiftStructParameters for reassembling structs on RISC-V This is what RISC-V did with old ABI classifiers --- src/coreclr/jit/abi.cpp | 14 ++++++++------ src/coreclr/jit/codegen.h | 2 +- src/coreclr/jit/codegencommon.cpp | 18 ++++++++++-------- src/coreclr/jit/codegenlinear.cpp | 8 ++++---- src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/flowgraph.cpp | 4 ++-- src/coreclr/jit/lclvars.cpp | 16 ++++++++++------ src/coreclr/jit/lsrabuild.cpp | 4 ++-- 8 files changed, 38 insertions(+), 30 deletions(-) diff --git a/src/coreclr/jit/abi.cpp b/src/coreclr/jit/abi.cpp index fd899b899546b1..2d97764814332b 100644 --- a/src/coreclr/jit/abi.cpp +++ b/src/coreclr/jit/abi.cpp @@ -258,14 +258,16 @@ bool ABIPassingInformation::HasExactlyOneStackSegment() const // bool ABIPassingInformation::IsSplitAcrossRegistersAndStack() const { - bool anyReg = false; - bool anyStack = false; - for (unsigned i = 0; i < NumSegments; i++) + if (NumSegments < 2) + return false; + + bool isFirstInReg = Segments[0].IsPassedInRegister(); + for (unsigned i = 1; i < NumSegments; i++) { - anyReg |= Segments[i].IsPassedInRegister(); - anyStack |= Segments[i].IsPassedOnStack(); + if (isFirstInReg != Segments[i].IsPassedInRegister()) + return true; } - return anyReg && anyStack; + return false; } //----------------------------------------------------------------------------- diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index 3511935a062b0a..fa0f880f522f12 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -274,7 +274,7 @@ class CodeGen final : public CodeGenInterface void genEnregisterOSRArgsAndLocals(); #endif - void genHomeSwiftStructParameters(bool handleStack); + void genHomeSplitStructParameters(bool handleStack); void genCheckUseBlockInit(); #if defined(UNIX_AMD64_ABI) && defined(FEATURE_SIMD) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index e446e638cfabb9..555eb1c3aa19ff 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -4119,24 +4119,26 @@ void CodeGen::genEnregisterOSRArgsAndLocals() } } -#ifdef SWIFT_SUPPORT +#if defined(SWIFT_SUPPORT) || defined(TARGET_RISCV64) //----------------------------------------------------------------------------- -// genHomeSwiftStructParameters: -// Reassemble Swift struct parameters if necessary. +// genHomeSplitStructParameters: +// Reassemble split struct parameters if necessary. // // Parameters: // handleStack - If true, reassemble the segments that were passed on the stack. // If false, reassemble the segments that were passed in registers. // -void CodeGen::genHomeSwiftStructParameters(bool handleStack) +void CodeGen::genHomeSplitStructParameters(bool handleStack) { for (unsigned lclNum = 0; lclNum < compiler->info.compArgsCount; lclNum++) { +#ifdef SWIFT_SUPPORT if (lclNum == compiler->lvaSwiftSelfArg) { continue; } +#endif LclVarDsc* dsc = compiler->lvaGetDesc(lclNum); if ((dsc->TypeGet() != TYP_STRUCT) || compiler->lvaIsImplicitByRefLocal(lclNum) || !dsc->lvOnFrame) @@ -4144,7 +4146,7 @@ void CodeGen::genHomeSwiftStructParameters(bool handleStack) continue; } - JITDUMP("Homing Swift parameter V%02u: ", lclNum); + JITDUMP("Homing split parameter V%02u: ", lclNum); const ABIPassingInformation& abiInfo = compiler->lvaGetParameterABIInfo(lclNum); DBEXEC(VERBOSE, abiInfo.Dump()); @@ -4174,7 +4176,7 @@ void CodeGen::genHomeSwiftStructParameters(bool handleStack) else { var_types loadType = TYP_UNDEF; - switch (seg.Size) + switch (RISCV64_ONLY(RoundUpToPower2)(seg.Size)) { case 1: loadType = TYP_UBYTE; @@ -4221,7 +4223,7 @@ void CodeGen::genHomeSwiftStructParameters(bool handleStack) } } } -#endif +#endif // defined(SWIFT_SUPPORT) || defined(TARGET_RISCV64) /*----------------------------------------------------------------------------- * @@ -5520,7 +5522,7 @@ void CodeGen::genFnProlog() intRegState.rsCalleeRegArgMaskLiveIn &= ~RBM_SWIFT_ERROR; } - genHomeSwiftStructParameters(/* handleStack */ false); + genHomeSplitStructParameters(/* handleStack */ false); } #endif diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index 145c074b8fc681..d88257ea3e4cff 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -386,13 +386,13 @@ void CodeGen::genCodeForBBlist() compiler->compCurStmt = nullptr; compiler->compCurLifeTree = nullptr; -#ifdef SWIFT_SUPPORT - // Reassemble Swift struct parameters on the local stack frame in the +#if defined(SWIFT_SUPPORT) || defined(TARGET_RISCV64) + // Reassemble split struct parameters on the local stack frame in the // scratch BB right after the prolog. There can be arbitrary amounts of // codegen related to doing this, so it cannot be done in the prolog. - if (compiler->fgBBisScratch(block) && compiler->lvaHasAnySwiftStackParamToReassemble()) + if (compiler->fgBBisScratch(block) && compiler->lvaHasAnyStackParamToReassemble()) { - genHomeSwiftStructParameters(/* handleStack */ true); + genHomeSplitStructParameters(/* handleStack */ true); } #endif diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 819c20629de134..77d96076a2501a 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -4002,7 +4002,7 @@ class Compiler void lvaClassifyParameterABI(); bool lvaInitSpecialSwiftParam(CORINFO_ARG_LIST_HANDLE argHnd, InitVarDscInfo* varDscInfo, CorInfoType type, CORINFO_CLASS_HANDLE typeHnd); - bool lvaHasAnySwiftStackParamToReassemble(); + bool lvaHasAnyStackParamToReassemble(); var_types lvaGetActualType(unsigned lclNum); var_types lvaGetRealType(unsigned lclNum); diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 6ab0977c55c252..1b90e7263dc4c9 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -2296,9 +2296,9 @@ PhaseStatus Compiler::fgAddInternal() madeChanges |= fgCreateFiltersForGenericExceptions(); // The backend requires a scratch BB into which it can safely insert a P/Invoke method prolog if one is - // required. Similarly, we need a scratch BB for poisoning and when we have Swift parameters to reassemble. + // required. Similarly, we need a scratch BB for poisoning and when we have split parameters to reassemble. // Create it here. - if (compMethodRequiresPInvokeFrame() || compShouldPoisonFrame() || lvaHasAnySwiftStackParamToReassemble()) + if (compMethodRequiresPInvokeFrame() || compShouldPoisonFrame() || lvaHasAnyStackParamToReassemble()) { madeChanges |= fgEnsureFirstBBisScratch(); fgFirstBB->SetFlags(BBF_DONT_REMOVE); diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp index c101955fdea7ed..8f58cad43115af 100644 --- a/src/coreclr/jit/lclvars.cpp +++ b/src/coreclr/jit/lclvars.cpp @@ -1889,31 +1889,35 @@ void Compiler::lvaClassifyParameterABI() } //-------------------------------------------------------------------------------------------- -// lvaHaveSwiftStructStackParamsToReassemble: -// Check if this compilation has any Swift parameters that are passed on the -// stack and that need to be reassembled on the local stack frame. +// lvaHasAnyStackParamToReassemble: +// Check if this compilation has any parameters split that need to be reassembled on the local stack frame. // // Return value: // True if so. // -bool Compiler::lvaHasAnySwiftStackParamToReassemble() +bool Compiler::lvaHasAnyStackParamToReassemble() { +#if defined(SWIFT_SUPPORT) || defined(TARGET_RISCV64) #ifdef SWIFT_SUPPORT if (info.compCallConv != CorInfoCallConvExtension::Swift) { return false; } +#endif for (unsigned lclNum = 0; lclNum < info.compArgsCount; lclNum++) { const ABIPassingInformation& abiInfo = lvaGetParameterABIInfo(lclNum); +#ifdef SWIFT_SUPPORT if (abiInfo.HasAnyStackSegment() && !abiInfo.HasExactlyOneStackSegment()) +#else // TARGET_RISCV64 + if (abiInfo.IsSplitAcrossRegistersAndStack()) +#endif { return true; } } -#endif - +#endif // defined(SWIFT_SUPPORT) || defined(TARGET_RISCV64) return false; } diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 3ee0f88ee2214f..6b48e2da4857c3 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -2542,12 +2542,12 @@ void LinearScan::buildIntervals() // assert(block->isRunRarely()); } - // For Swift calls there can be an arbitrary amount of codegen related + // For Swift and RISC-V calls there can be an arbitrary amount of codegen related // to homing of decomposed struct parameters passed on stack. We cannot // do that in the prolog. We handle registers in the prolog and the // stack args in the scratch BB that we have ensured exists. The // handling clobbers REG_SCRATCH, so kill it here. - if ((block == compiler->fgFirstBB) && compiler->lvaHasAnySwiftStackParamToReassemble()) + if ((block == compiler->fgFirstBB) && compiler->lvaHasAnyStackParamToReassemble()) { assert(compiler->fgFirstBBisScratch()); addRefsForPhysRegMask(genRegMask(REG_SCRATCH), currentLoc + 1, RefTypeKill, true); From 85c2217500e14239143ce750558752f9edc1ce66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Thu, 25 Apr 2024 15:48:03 +0200 Subject: [PATCH 05/14] Code review --- src/coreclr/jit/codegen.h | 2 +- src/coreclr/jit/codegencommon.cpp | 16 ++++++++++------ src/coreclr/jit/codegenlinear.cpp | 2 +- src/coreclr/jit/lclvars.cpp | 17 ++++++++++------- 4 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index fa0f880f522f12..f1dde690333563 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -274,7 +274,7 @@ class CodeGen final : public CodeGenInterface void genEnregisterOSRArgsAndLocals(); #endif - void genHomeSplitStructParameters(bool handleStack); + void genHomeReassembledParameters(bool handleStack); void genCheckUseBlockInit(); #if defined(UNIX_AMD64_ABI) && defined(FEATURE_SIMD) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 555eb1c3aa19ff..4a534492d66357 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -4122,14 +4122,14 @@ void CodeGen::genEnregisterOSRArgsAndLocals() #if defined(SWIFT_SUPPORT) || defined(TARGET_RISCV64) //----------------------------------------------------------------------------- -// genHomeSplitStructParameters: -// Reassemble split struct parameters if necessary. +// genHomeReassembledParameters: +// Reassemble parameters if necessary. // // Parameters: // handleStack - If true, reassemble the segments that were passed on the stack. // If false, reassemble the segments that were passed in registers. // -void CodeGen::genHomeSplitStructParameters(bool handleStack) +void CodeGen::genHomeReassembledParameters(bool handleStack) { for (unsigned lclNum = 0; lclNum < compiler->info.compArgsCount; lclNum++) { @@ -4146,7 +4146,7 @@ void CodeGen::genHomeSplitStructParameters(bool handleStack) continue; } - JITDUMP("Homing split parameter V%02u: ", lclNum); + JITDUMP("Homing reassembled parameter V%02u: ", lclNum); const ABIPassingInformation& abiInfo = compiler->lvaGetParameterABIInfo(lclNum); DBEXEC(VERBOSE, abiInfo.Dump()); @@ -4176,7 +4176,7 @@ void CodeGen::genHomeSplitStructParameters(bool handleStack) else { var_types loadType = TYP_UNDEF; - switch (RISCV64_ONLY(RoundUpToPower2)(seg.Size)) + switch (seg.Size) { case 1: loadType = TYP_UBYTE; @@ -4184,9 +4184,13 @@ void CodeGen::genHomeSplitStructParameters(bool handleStack) case 2: loadType = TYP_USHORT; break; + case 3: case 4: loadType = TYP_INT; break; + case 5: + case 6: + case 7: case 8: loadType = TYP_LONG; break; @@ -5522,7 +5526,7 @@ void CodeGen::genFnProlog() intRegState.rsCalleeRegArgMaskLiveIn &= ~RBM_SWIFT_ERROR; } - genHomeSplitStructParameters(/* handleStack */ false); + genHomeReassembledParameters(/* handleStack */ false); } #endif diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index d88257ea3e4cff..59a21242be73be 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -392,7 +392,7 @@ void CodeGen::genCodeForBBlist() // codegen related to doing this, so it cannot be done in the prolog. if (compiler->fgBBisScratch(block) && compiler->lvaHasAnyStackParamToReassemble()) { - genHomeSplitStructParameters(/* handleStack */ true); + genHomeReassembledParameters(/* handleStack */ true); } #endif diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp index 8f58cad43115af..9c809ecb08327f 100644 --- a/src/coreclr/jit/lclvars.cpp +++ b/src/coreclr/jit/lclvars.cpp @@ -1897,27 +1897,30 @@ void Compiler::lvaClassifyParameterABI() // bool Compiler::lvaHasAnyStackParamToReassemble() { -#if defined(SWIFT_SUPPORT) || defined(TARGET_RISCV64) -#ifdef SWIFT_SUPPORT +#if defined(SWIFT_SUPPORT) if (info.compCallConv != CorInfoCallConvExtension::Swift) { return false; } -#endif for (unsigned lclNum = 0; lclNum < info.compArgsCount; lclNum++) { const ABIPassingInformation& abiInfo = lvaGetParameterABIInfo(lclNum); -#ifdef SWIFT_SUPPORT if (abiInfo.HasAnyStackSegment() && !abiInfo.HasExactlyOneStackSegment()) -#else // TARGET_RISCV64 + { + return true; + } + } +#elif defined(TARGET_RISCV64) + for (unsigned lclNum = 0; lclNum < info.compArgsCount; lclNum++) + { + const ABIPassingInformation& abiInfo = lvaGetParameterABIInfo(lclNum); if (abiInfo.IsSplitAcrossRegistersAndStack()) -#endif { return true; } } -#endif // defined(SWIFT_SUPPORT) || defined(TARGET_RISCV64) +#endif return false; } From 5a0aedef11ba3227c60b4610f214bb175530dcb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Thu, 25 Apr 2024 16:12:35 +0200 Subject: [PATCH 06/14] genHome(Reassembled=>Stack)Parameters rename --- src/coreclr/jit/codegen.h | 2 +- src/coreclr/jit/codegencommon.cpp | 6 +++--- src/coreclr/jit/codegenlinear.cpp | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index 181b92f4827eb7..257d21021502a4 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -284,7 +284,7 @@ class CodeGen final : public CodeGenInterface void genEnregisterOSRArgsAndLocals(); #endif - void genHomeReassembledParameters(bool handleStack); + void genHomeStackParams(bool handleStack); void genCheckUseBlockInit(); #if defined(UNIX_AMD64_ABI) && defined(FEATURE_SIMD) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index d22d1b29be8103..c5bcb125373226 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -4122,14 +4122,14 @@ void CodeGen::genEnregisterOSRArgsAndLocals() #if defined(SWIFT_SUPPORT) || defined(TARGET_RISCV64) //----------------------------------------------------------------------------- -// genHomeReassembledParameters: +// genHomeStackParams: // Reassemble parameters if necessary. // // Parameters: // handleStack - If true, reassemble the segments that were passed on the stack. // If false, reassemble the segments that were passed in registers. // -void CodeGen::genHomeReassembledParameters(bool handleStack) +void CodeGen::genHomeStackParams(bool handleStack) { for (unsigned lclNum = 0; lclNum < compiler->info.compArgsCount; lclNum++) { @@ -5519,7 +5519,7 @@ void CodeGen::genFnProlog() intRegState.rsCalleeRegArgMaskLiveIn &= ~RBM_SWIFT_ERROR; } - genHomeReassembledParameters(/* handleStack */ false); + genHomeStackParams(/* handleStack */ false); } #endif diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index d47a0acf497cca..320c82170c0eac 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -392,7 +392,7 @@ void CodeGen::genCodeForBBlist() // codegen related to doing this, so it cannot be done in the prolog. if (compiler->fgBBisScratch(block) && compiler->lvaHasAnyStackParamToReassemble()) { - genHomeReassembledParameters(/* handleStack */ true); + genHomeStackParams(/* handleStack */ true); } #endif From 480647803953d3581210c8b38cf38463d26bbab8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Thu, 25 Apr 2024 17:16:24 +0200 Subject: [PATCH 07/14] Call genHomeStackParams(true) in prolog generation for RISC-V to avoid garbage GC pointers --- src/coreclr/jit/codegencommon.cpp | 7 +++++++ src/coreclr/jit/codegenlinear.cpp | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index c5bcb125373226..06ea36515c8595 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -5555,6 +5555,13 @@ void CodeGen::genFnProlog() genEnregisterIncomingStackArgs(); } +#ifdef TARGET_RISCV64 + if (compiler->fgFirstBBisScratch() && compiler->lvaHasAnyStackParamToReassemble()) + { + genHomeStackParams(/* handleStack */ true); + } +#endif + /* Initialize any must-init registers variables now */ if (initRegs) diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index 320c82170c0eac..32e20061e652c5 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -386,7 +386,7 @@ void CodeGen::genCodeForBBlist() compiler->compCurStmt = nullptr; compiler->compCurLifeTree = nullptr; -#if defined(SWIFT_SUPPORT) || defined(TARGET_RISCV64) +#ifdef SWIFT_SUPPORT // Reassemble split struct parameters on the local stack frame in the // scratch BB right after the prolog. There can be arbitrary amounts of // codegen related to doing this, so it cannot be done in the prolog. From 123fcee39846b310da11b7a29daa1f52241780c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Fri, 26 Apr 2024 08:17:49 +0200 Subject: [PATCH 08/14] Leave Swift codes alone This reverts commits 480647803953d3581210c8b38cf38463d26bbab8 5a0aedef11ba3227c60b4610f214bb175530dcb0 85c2217500e14239143ce750558752f9edc1ce66 06e9d6548130c0918a0301bd662c42152ae765f4 --- src/coreclr/jit/codegen.h | 2 +- src/coreclr/jit/codegencommon.cpp | 27 +++++++-------------------- src/coreclr/jit/codegenlinear.cpp | 6 +++--- src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/flowgraph.cpp | 4 ++-- src/coreclr/jit/lclvars.cpp | 19 ++++++------------- src/coreclr/jit/lsrabuild.cpp | 4 ++-- 7 files changed, 22 insertions(+), 42 deletions(-) diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index 257d21021502a4..441db5f6b50d47 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -284,7 +284,7 @@ class CodeGen final : public CodeGenInterface void genEnregisterOSRArgsAndLocals(); #endif - void genHomeStackParams(bool handleStack); + void genHomeSwiftStructParameters(bool handleStack); void genCheckUseBlockInit(); #if defined(UNIX_AMD64_ABI) && defined(FEATURE_SIMD) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 06ea36515c8595..c528bb28bed1fc 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -4119,26 +4119,24 @@ void CodeGen::genEnregisterOSRArgsAndLocals() } } -#if defined(SWIFT_SUPPORT) || defined(TARGET_RISCV64) +#ifdef SWIFT_SUPPORT //----------------------------------------------------------------------------- -// genHomeStackParams: -// Reassemble parameters if necessary. +// genHomeSwiftStructParameters: +// Reassemble Swift struct parameters if necessary. // // Parameters: // handleStack - If true, reassemble the segments that were passed on the stack. // If false, reassemble the segments that were passed in registers. // -void CodeGen::genHomeStackParams(bool handleStack) +void CodeGen::genHomeSwiftStructParameters(bool handleStack) { for (unsigned lclNum = 0; lclNum < compiler->info.compArgsCount; lclNum++) { -#ifdef SWIFT_SUPPORT if (lclNum == compiler->lvaSwiftSelfArg) { continue; } -#endif LclVarDsc* dsc = compiler->lvaGetDesc(lclNum); if ((dsc->TypeGet() != TYP_STRUCT) || compiler->lvaIsImplicitByRefLocal(lclNum) || !dsc->lvOnFrame) @@ -4146,7 +4144,7 @@ void CodeGen::genHomeStackParams(bool handleStack) continue; } - JITDUMP("Homing reassembled parameter V%02u: ", lclNum); + JITDUMP("Homing Swift parameter V%02u: ", lclNum); const ABIPassingInformation& abiInfo = compiler->lvaGetParameterABIInfo(lclNum); DBEXEC(VERBOSE, abiInfo.Dump()); @@ -4184,13 +4182,9 @@ void CodeGen::genHomeStackParams(bool handleStack) case 2: loadType = TYP_USHORT; break; - case 3: case 4: loadType = TYP_INT; break; - case 5: - case 6: - case 7: case 8: loadType = TYP_LONG; break; @@ -4227,7 +4221,7 @@ void CodeGen::genHomeStackParams(bool handleStack) } } } -#endif // defined(SWIFT_SUPPORT) || defined(TARGET_RISCV64) +#endif /*----------------------------------------------------------------------------- * @@ -5519,7 +5513,7 @@ void CodeGen::genFnProlog() intRegState.rsCalleeRegArgMaskLiveIn &= ~RBM_SWIFT_ERROR; } - genHomeStackParams(/* handleStack */ false); + genHomeSwiftStructParameters(/* handleStack */ false); } #endif @@ -5555,13 +5549,6 @@ void CodeGen::genFnProlog() genEnregisterIncomingStackArgs(); } -#ifdef TARGET_RISCV64 - if (compiler->fgFirstBBisScratch() && compiler->lvaHasAnyStackParamToReassemble()) - { - genHomeStackParams(/* handleStack */ true); - } -#endif - /* Initialize any must-init registers variables now */ if (initRegs) diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index 32e20061e652c5..d099fe192fc38a 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -387,12 +387,12 @@ void CodeGen::genCodeForBBlist() compiler->compCurLifeTree = nullptr; #ifdef SWIFT_SUPPORT - // Reassemble split struct parameters on the local stack frame in the + // Reassemble Swift struct parameters on the local stack frame in the // scratch BB right after the prolog. There can be arbitrary amounts of // codegen related to doing this, so it cannot be done in the prolog. - if (compiler->fgBBisScratch(block) && compiler->lvaHasAnyStackParamToReassemble()) + if (compiler->fgBBisScratch(block) && compiler->lvaHasAnySwiftStackParamToReassemble()) { - genHomeStackParams(/* handleStack */ true); + genHomeSwiftStructParameters(/* handleStack */ true); } #endif diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 37d298abf11e41..7949058066494d 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -4007,7 +4007,7 @@ class Compiler void lvaClassifyParameterABI(); bool lvaInitSpecialSwiftParam(CORINFO_ARG_LIST_HANDLE argHnd, InitVarDscInfo* varDscInfo, CorInfoType type, CORINFO_CLASS_HANDLE typeHnd); - bool lvaHasAnyStackParamToReassemble(); + bool lvaHasAnySwiftStackParamToReassemble(); var_types lvaGetActualType(unsigned lclNum); var_types lvaGetRealType(unsigned lclNum); diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 1b90e7263dc4c9..6ab0977c55c252 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -2296,9 +2296,9 @@ PhaseStatus Compiler::fgAddInternal() madeChanges |= fgCreateFiltersForGenericExceptions(); // The backend requires a scratch BB into which it can safely insert a P/Invoke method prolog if one is - // required. Similarly, we need a scratch BB for poisoning and when we have split parameters to reassemble. + // required. Similarly, we need a scratch BB for poisoning and when we have Swift parameters to reassemble. // Create it here. - if (compMethodRequiresPInvokeFrame() || compShouldPoisonFrame() || lvaHasAnyStackParamToReassemble()) + if (compMethodRequiresPInvokeFrame() || compShouldPoisonFrame() || lvaHasAnySwiftStackParamToReassemble()) { madeChanges |= fgEnsureFirstBBisScratch(); fgFirstBB->SetFlags(BBF_DONT_REMOVE); diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp index e4492ef8470c3f..e6645e1f03d8f4 100644 --- a/src/coreclr/jit/lclvars.cpp +++ b/src/coreclr/jit/lclvars.cpp @@ -1889,15 +1889,16 @@ void Compiler::lvaClassifyParameterABI() } //-------------------------------------------------------------------------------------------- -// lvaHasAnyStackParamToReassemble: -// Check if this compilation has any parameters split that need to be reassembled on the local stack frame. +// lvaHaveSwiftStructStackParamsToReassemble: +// Check if this compilation has any Swift parameters that are passed on the +// stack and that need to be reassembled on the local stack frame. // // Return value: // True if so. // -bool Compiler::lvaHasAnyStackParamToReassemble() +bool Compiler::lvaHasAnySwiftStackParamToReassemble() { -#if defined(SWIFT_SUPPORT) +#ifdef SWIFT_SUPPORT if (info.compCallConv != CorInfoCallConvExtension::Swift) { return false; @@ -1911,16 +1912,8 @@ bool Compiler::lvaHasAnyStackParamToReassemble() return true; } } -#elif defined(TARGET_RISCV64) - for (unsigned lclNum = 0; lclNum < info.compArgsCount; lclNum++) - { - const ABIPassingInformation& abiInfo = lvaGetParameterABIInfo(lclNum); - if (abiInfo.IsSplitAcrossRegistersAndStack()) - { - return true; - } - } #endif + return false; } diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index a7a2f4be184dbb..37fa4320cc6918 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -2542,12 +2542,12 @@ void LinearScan::buildIntervals() // assert(block->isRunRarely()); } - // For Swift and RISC-V calls there can be an arbitrary amount of codegen related + // For Swift calls there can be an arbitrary amount of codegen related // to homing of decomposed struct parameters passed on stack. We cannot // do that in the prolog. We handle registers in the prolog and the // stack args in the scratch BB that we have ensured exists. The // handling clobbers REG_SCRATCH, so kill it here. - if ((block == compiler->fgFirstBB) && compiler->lvaHasAnyStackParamToReassemble()) + if ((block == compiler->fgFirstBB) && compiler->lvaHasAnySwiftStackParamToReassemble()) { assert(compiler->fgFirstBBisScratch()); addRefsForPhysRegMask(genRegMask(REG_SCRATCH), currentLoc + 1, RefTypeKill, true); From 9c565acd4901c5157dbee38ae0d37606c0a8115b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Fri, 26 Apr 2024 13:44:09 +0200 Subject: [PATCH 09/14] Make a RISC-V specific routine for homing stack parts of split parameters. --- src/coreclr/jit/codegen.h | 2 ++ src/coreclr/jit/codegencommon.cpp | 37 +++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index 441db5f6b50d47..a7d355e7cdd1f7 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -286,6 +286,8 @@ class CodeGen final : public CodeGenInterface void genHomeSwiftStructParameters(bool handleStack); + void genHomeStackPartOfSplitParameter(unsigned lclNum); + void genCheckUseBlockInit(); #if defined(UNIX_AMD64_ABI) && defined(FEATURE_SIMD) void genClearStackVec3ArgUpperBits(); diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index c528bb28bed1fc..038c8128722b12 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -3068,6 +3068,7 @@ void CodeGen::genSpillOrAddRegisterParam(unsigned lclNum, RegGraph* graph) unsigned baseOffset = varDsc->lvIsStructField ? varDsc->lvFldOffset : 0; unsigned size = varDsc->lvExactSize(); + bool isSpilled = false; unsigned paramLclNum = varDsc->lvIsStructField ? varDsc->lvParentLcl : lclNum; LclVarDsc* paramVarDsc = compiler->lvaGetDesc(paramLclNum); @@ -3101,6 +3102,7 @@ void CodeGen::genSpillOrAddRegisterParam(unsigned lclNum, RegGraph* graph) GetEmitter()->emitIns_S_R(ins_Store(storeType), emitActualTypeSize(storeType), seg.GetRegister(), lclNum, seg.Offset - baseOffset); + isSpilled = true; } if (!varDsc->lvIsInReg()) @@ -3137,6 +3139,11 @@ void CodeGen::genSpillOrAddRegisterParam(unsigned lclNum, RegGraph* graph) graph->AddEdge(sourceReg, destReg, edgeType, seg.Offset - baseOffset); } } + + if (isSpilled) + { + genHomeStackPartOfSplitParameter(lclNum); + } } // ----------------------------------------------------------------------------- @@ -3156,6 +3163,8 @@ void CodeGen::genHomeRegisterParams(regNumber initReg, bool* initRegStillZeroed) } #endif + // RISC-V specific logic for homing the stack part of a split struct + regMaskTP paramRegs = intRegState.rsCalleeRegArgMaskLiveIn | floatRegState.rsCalleeRegArgMaskLiveIn; if (compiler->opts.OptimizationDisabled()) { @@ -3180,6 +3189,8 @@ void CodeGen::genHomeRegisterParams(regNumber initReg, bool* initRegStillZeroed) lclNum, seg.Offset); } } + + genHomeStackPartOfSplitParameter(lclNum); } return; @@ -4223,6 +4234,32 @@ void CodeGen::genHomeSwiftStructParameters(bool handleStack) } #endif +/*----------------------------------------------------------------------------- + * + * Home the tail (stack) portion of a split parameter next to where the head (register) portion is homed. + * + * No-op on platforms where argument registers are already homed to form a contiguous space with incoming stack. + */ +void CodeGen::genHomeStackPartOfSplitParameter(unsigned lclNum) +{ +#ifdef TARGET_RISCV64 + const LclVarDsc* var = compiler->lvaGetDesc(lclNum); + if (!var->lvIsSplit) + return; + + assert(varTypeIsStruct(var)); + const ABIPassingInformation& abiInfo = compiler->lvaGetParameterABIInfo(lclNum); + assert(abiInfo.NumSegments == 2); + assert(abiInfo.Segments[0].GetRegister() == REG_ARG_LAST); + const ABIPassingSegment& seg = abiInfo.Segments[1]; + + int loadOffset = + -(isFramePointerUsed() ? genCallerSPtoFPdelta() : genCallerSPtoInitialSPdelta()) + (int)seg.GetStackOffset(); + genInstrWithConstant(ins_Load(TYP_LONG), EA_8BYTE, REG_SCRATCH, genFramePointerReg(), loadOffset, REG_SCRATCH); + GetEmitter()->emitIns_S_R(ins_Store(TYP_LONG), EA_8BYTE, REG_SCRATCH, lclNum, seg.Offset); +#endif +} + /*----------------------------------------------------------------------------- * * Save the generic context argument. From c86cb0996712183b2603695ee2dafe6083dcc701 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Fri, 26 Apr 2024 19:08:40 +0200 Subject: [PATCH 10/14] Move genHomeStackPartOfSplitParameter out of genHomeSwiftStructParameters, share stack segment homing with Swift code --- src/coreclr/jit/codegen.h | 4 +- src/coreclr/jit/codegencommon.cpp | 140 ++++++++++++++++-------------- 2 files changed, 75 insertions(+), 69 deletions(-) diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index a7d355e7cdd1f7..2a2ede6bae9ac0 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -284,9 +284,9 @@ class CodeGen final : public CodeGenInterface void genEnregisterOSRArgsAndLocals(); #endif + void genHomeStackSegment(unsigned lclNum, const ABIPassingSegment& seg, regNumber initReg, bool* pInitRegZeroed); void genHomeSwiftStructParameters(bool handleStack); - - void genHomeStackPartOfSplitParameter(unsigned lclNum); + void genHomeStackPartOfSplitParameter(regNumber initReg, bool* initRegStillZeroed); void genCheckUseBlockInit(); #if defined(UNIX_AMD64_ABI) && defined(FEATURE_SIMD) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 038c8128722b12..341cff68ab8826 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -3068,7 +3068,6 @@ void CodeGen::genSpillOrAddRegisterParam(unsigned lclNum, RegGraph* graph) unsigned baseOffset = varDsc->lvIsStructField ? varDsc->lvFldOffset : 0; unsigned size = varDsc->lvExactSize(); - bool isSpilled = false; unsigned paramLclNum = varDsc->lvIsStructField ? varDsc->lvParentLcl : lclNum; LclVarDsc* paramVarDsc = compiler->lvaGetDesc(paramLclNum); @@ -3102,7 +3101,6 @@ void CodeGen::genSpillOrAddRegisterParam(unsigned lclNum, RegGraph* graph) GetEmitter()->emitIns_S_R(ins_Store(storeType), emitActualTypeSize(storeType), seg.GetRegister(), lclNum, seg.Offset - baseOffset); - isSpilled = true; } if (!varDsc->lvIsInReg()) @@ -3139,11 +3137,6 @@ void CodeGen::genSpillOrAddRegisterParam(unsigned lclNum, RegGraph* graph) graph->AddEdge(sourceReg, destReg, edgeType, seg.Offset - baseOffset); } } - - if (isSpilled) - { - genHomeStackPartOfSplitParameter(lclNum); - } } // ----------------------------------------------------------------------------- @@ -3189,8 +3182,6 @@ void CodeGen::genHomeRegisterParams(regNumber initReg, bool* initRegStillZeroed) lclNum, seg.Offset); } } - - genHomeStackPartOfSplitParameter(lclNum); } return; @@ -4130,6 +4121,51 @@ void CodeGen::genEnregisterOSRArgsAndLocals() } } +#if defined(SWIFT_SUPPORT) || defined(TARGET_RISCV64) +/*----------------------------------------------------------------------------- + * Move the incoming segment to the local stack frame + */ +void CodeGen::genHomeStackSegment(unsigned lclNum, const ABIPassingSegment& seg, regNumber initReg, bool* initRegStillZeroed) +{ + var_types loadType = TYP_UNDEF; + switch (seg.Size) + { + case 1: + loadType = TYP_UBYTE; + break; + case 2: + loadType = TYP_USHORT; + break; + case 3: + case 4: + loadType = TYP_INT; + break; + case 5: + case 6: + case 7: + case 8: + loadType = TYP_LONG; + break; + default: + assert(!"Unexpected segment size for struct parameter not passed implicitly by ref"); + return; + } + emitAttr size = emitTypeSize(loadType); + + int loadOffset = + -(isFramePointerUsed() ? genCallerSPtoFPdelta() : genCallerSPtoInitialSPdelta()) + (int)seg.GetStackOffset(); +#ifdef TARGET_XARCH + GetEmitter()->emitIns_R_AR(ins_Load(loadType), size, initReg, genFramePointerReg(), offset); +#else + genInstrWithConstant(ins_Load(loadType), size, initReg, genFramePointerReg(), loadOffset, initReg); +#endif + GetEmitter()->emitIns_S_R(ins_Store(loadType), size, initReg, lclNum, seg.Offset); + + if (initRegStillZeroed) + *initRegStillZeroed = false; +} +#endif // defined(SWIFT_SUPPORT) || defined(TARGET_RISCV64) + #ifdef SWIFT_SUPPORT //----------------------------------------------------------------------------- @@ -4161,7 +4197,7 @@ void CodeGen::genHomeSwiftStructParameters(bool handleStack) for (unsigned i = 0; i < abiInfo.NumSegments; i++) { - const ABIPassingSegment& seg = abiInfo.Segments[i]; + const ABIPassingSegment& seg = abiInfo.Segments[i]; if (seg.IsPassedOnStack() != handleStack) { continue; @@ -4184,50 +4220,8 @@ void CodeGen::genHomeSwiftStructParameters(bool handleStack) } else { - var_types loadType = TYP_UNDEF; - switch (seg.Size) - { - case 1: - loadType = TYP_UBYTE; - break; - case 2: - loadType = TYP_USHORT; - break; - case 4: - loadType = TYP_INT; - break; - case 8: - loadType = TYP_LONG; - break; - default: - assert(!"Unexpected segment size for struct parameter not passed implicitly by ref"); - continue; - } - - int offset; - if (isFramePointerUsed()) - { - offset = -genCallerSPtoFPdelta(); - } - else - { - offset = -genCallerSPtoInitialSPdelta(); - } - - offset += (int)seg.GetStackOffset(); - - // Move the incoming segment to the local stack frame. We can - // use REG_SCRATCH as a temporary register here as we ensured - // that during LSRA build. -#ifdef TARGET_XARCH - GetEmitter()->emitIns_R_AR(ins_Load(loadType), emitTypeSize(loadType), REG_SCRATCH, - genFramePointerReg(), offset); -#else - genInstrWithConstant(ins_Load(loadType), emitTypeSize(loadType), REG_SCRATCH, genFramePointerReg(), - offset, REG_SCRATCH); -#endif - - GetEmitter()->emitIns_S_R(ins_Store(loadType), emitTypeSize(loadType), REG_SCRATCH, lclNum, seg.Offset); + // We can use REG_SCRATCH as a temporary register here as we ensured that during LSRA build. + genHomeStackSegment(lclNum, seg, REG_SCRATCH, nullptr); } } } @@ -4240,23 +4234,34 @@ void CodeGen::genHomeSwiftStructParameters(bool handleStack) * * No-op on platforms where argument registers are already homed to form a contiguous space with incoming stack. */ -void CodeGen::genHomeStackPartOfSplitParameter(unsigned lclNum) +void CodeGen::genHomeStackPartOfSplitParameter(regNumber initReg, bool* initRegStillZeroed) { #ifdef TARGET_RISCV64 - const LclVarDsc* var = compiler->lvaGetDesc(lclNum); - if (!var->lvIsSplit) - return; + unsigned lclNum = 0; + for (; lclNum < compiler->info.compArgsCount; lclNum++) + { + LclVarDsc* var = compiler->lvaGetDesc(lclNum); + if (!var->lvIsSplit) + continue; - assert(varTypeIsStruct(var)); - const ABIPassingInformation& abiInfo = compiler->lvaGetParameterABIInfo(lclNum); - assert(abiInfo.NumSegments == 2); - assert(abiInfo.Segments[0].GetRegister() == REG_ARG_LAST); - const ABIPassingSegment& seg = abiInfo.Segments[1]; + JITDUMP("Homing stack part of split parameter V%02u\n", lclNum); - int loadOffset = - -(isFramePointerUsed() ? genCallerSPtoFPdelta() : genCallerSPtoInitialSPdelta()) + (int)seg.GetStackOffset(); - genInstrWithConstant(ins_Load(TYP_LONG), EA_8BYTE, REG_SCRATCH, genFramePointerReg(), loadOffset, REG_SCRATCH); - GetEmitter()->emitIns_S_R(ins_Store(TYP_LONG), EA_8BYTE, REG_SCRATCH, lclNum, seg.Offset); + assert(varTypeIsStruct(var)); + assert(var->lvOnFrame); + assert(!compiler->lvaIsImplicitByRefLocal(lclNum)); + const ABIPassingInformation& abiInfo = compiler->lvaGetParameterABIInfo(lclNum); + assert(abiInfo.NumSegments == 2); + assert(abiInfo.Segments[0].GetRegister() == REG_ARG_LAST); + const ABIPassingSegment& seg = abiInfo.Segments[1]; + + genHomeStackSegment(lclNum, seg, initReg, initRegStillZeroed); + + for (lclNum+=1; lclNum < compiler->info.compArgsCount; lclNum++) + { + assert(!compiler->lvaGetDesc(lclNum)->lvIsSplit); // There should be only one split parameter + } + break; + } #endif } @@ -5580,6 +5585,7 @@ void CodeGen::genFnProlog() if ((intRegState.rsCalleeRegArgMaskLiveIn | floatRegState.rsCalleeRegArgMaskLiveIn) != RBM_NONE) { genHomeRegisterParams(initReg, &initRegZeroed); + genHomeStackPartOfSplitParameter(initReg, &initRegZeroed); } // Home the incoming arguments. From 5225a43b2f423aa50d1f514113e93b72f1a7f101 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Fri, 26 Apr 2024 19:41:27 +0200 Subject: [PATCH 11/14] fix x64 build --- src/coreclr/jit/codegencommon.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 341cff68ab8826..40880ddcf64d84 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -4155,7 +4155,7 @@ void CodeGen::genHomeStackSegment(unsigned lclNum, const ABIPassingSegment& seg, int loadOffset = -(isFramePointerUsed() ? genCallerSPtoFPdelta() : genCallerSPtoInitialSPdelta()) + (int)seg.GetStackOffset(); #ifdef TARGET_XARCH - GetEmitter()->emitIns_R_AR(ins_Load(loadType), size, initReg, genFramePointerReg(), offset); + GetEmitter()->emitIns_R_AR(ins_Load(loadType), size, initReg, genFramePointerReg(), loadOffset); #else genInstrWithConstant(ins_Load(loadType), size, initReg, genFramePointerReg(), loadOffset, initReg); #endif From f7b0bcca1a76de985ea333596d5d9a0e84895975 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Fri, 26 Apr 2024 19:44:07 +0200 Subject: [PATCH 12/14] format --- src/coreclr/jit/codegencommon.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 40880ddcf64d84..843d99ad506e78 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -4125,7 +4125,10 @@ void CodeGen::genEnregisterOSRArgsAndLocals() /*----------------------------------------------------------------------------- * Move the incoming segment to the local stack frame */ -void CodeGen::genHomeStackSegment(unsigned lclNum, const ABIPassingSegment& seg, regNumber initReg, bool* initRegStillZeroed) +void CodeGen::genHomeStackSegment(unsigned lclNum, + const ABIPassingSegment& seg, + regNumber initReg, + bool* initRegStillZeroed) { var_types loadType = TYP_UNDEF; switch (seg.Size) @@ -4197,7 +4200,7 @@ void CodeGen::genHomeSwiftStructParameters(bool handleStack) for (unsigned i = 0; i < abiInfo.NumSegments; i++) { - const ABIPassingSegment& seg = abiInfo.Segments[i]; + const ABIPassingSegment& seg = abiInfo.Segments[i]; if (seg.IsPassedOnStack() != handleStack) { continue; @@ -4256,7 +4259,7 @@ void CodeGen::genHomeStackPartOfSplitParameter(regNumber initReg, bool* initRegS genHomeStackSegment(lclNum, seg, initReg, initRegStillZeroed); - for (lclNum+=1; lclNum < compiler->info.compArgsCount; lclNum++) + for (lclNum += 1; lclNum < compiler->info.compArgsCount; lclNum++) { assert(!compiler->lvaGetDesc(lclNum)->lvIsSplit); // There should be only one split parameter } From bfaf9ce2bae28612ea4cdf5c9c06001dc1b4e0cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Fri, 26 Apr 2024 20:07:59 +0200 Subject: [PATCH 13/14] review --- src/coreclr/jit/codegencommon.cpp | 43 +++++++++++++++++++------------ 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 843d99ad506e78..09ea2ee325cb11 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -4122,9 +4122,15 @@ void CodeGen::genEnregisterOSRArgsAndLocals() } #if defined(SWIFT_SUPPORT) || defined(TARGET_RISCV64) -/*----------------------------------------------------------------------------- - * Move the incoming segment to the local stack frame - */ +//----------------------------------------------------------------------------- +// genHomeSwiftStructParameters: Move the incoming segment to the local stack frame. +// +// Arguments: +// lclNum - Number of local variable to home +// seg - Stack segment of the local variable to home +// initReg - Scratch register to use if needed +// initRegStillZeroed - Set to false if the scratch register was needed +// void CodeGen::genHomeStackSegment(unsigned lclNum, const ABIPassingSegment& seg, regNumber initReg, @@ -4173,11 +4179,11 @@ void CodeGen::genHomeStackSegment(unsigned lclNum, //----------------------------------------------------------------------------- // genHomeSwiftStructParameters: -// Reassemble Swift struct parameters if necessary. +// Reassemble Swift struct parameters if necessary. // -// Parameters: -// handleStack - If true, reassemble the segments that were passed on the stack. -// If false, reassemble the segments that were passed in registers. +// Arguments: +// handleStack - If true, reassemble the segments that were passed on the stack. +// If false, reassemble the segments that were passed in registers. // void CodeGen::genHomeSwiftStructParameters(bool handleStack) { @@ -4231,12 +4237,17 @@ void CodeGen::genHomeSwiftStructParameters(bool handleStack) } #endif -/*----------------------------------------------------------------------------- - * - * Home the tail (stack) portion of a split parameter next to where the head (register) portion is homed. - * - * No-op on platforms where argument registers are already homed to form a contiguous space with incoming stack. - */ +//----------------------------------------------------------------------------- +// genHomeStackPartOfSplitParameter: Home the tail (stack) portion of a split parameter next to where the head +// (register) portion is homed. +// +// Arguments: +// initReg - scratch register to use if needed +// initRegStillZeroed - set to false if scratch register was needed +// +// Notes: +// No-op on platforms where argument registers are already homed to form a contiguous space with incoming stack. +// void CodeGen::genHomeStackPartOfSplitParameter(regNumber initReg, bool* initRegStillZeroed) { #ifdef TARGET_RISCV64 @@ -4244,13 +4255,12 @@ void CodeGen::genHomeStackPartOfSplitParameter(regNumber initReg, bool* initRegS for (; lclNum < compiler->info.compArgsCount; lclNum++) { LclVarDsc* var = compiler->lvaGetDesc(lclNum); - if (!var->lvIsSplit) + if (!var->lvIsSplit || !var->lvOnFrame) continue; JITDUMP("Homing stack part of split parameter V%02u\n", lclNum); assert(varTypeIsStruct(var)); - assert(var->lvOnFrame); assert(!compiler->lvaIsImplicitByRefLocal(lclNum)); const ABIPassingInformation& abiInfo = compiler->lvaGetParameterABIInfo(lclNum); assert(abiInfo.NumSegments == 2); @@ -5585,10 +5595,11 @@ void CodeGen::genFnProlog() { compiler->lvaUpdateArgsWithInitialReg(); + genHomeStackPartOfSplitParameter(initReg, &initRegZeroed); + if ((intRegState.rsCalleeRegArgMaskLiveIn | floatRegState.rsCalleeRegArgMaskLiveIn) != RBM_NONE) { genHomeRegisterParams(initReg, &initRegZeroed); - genHomeStackPartOfSplitParameter(initReg, &initRegZeroed); } // Home the incoming arguments. From 2d2717ecf95c86335202e7e8c7c3784ab736e06c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Sowi=C5=84ski?= Date: Fri, 26 Apr 2024 20:16:48 +0200 Subject: [PATCH 14/14] remove comment Co-authored-by: Jakob Botsch Nielsen --- src/coreclr/jit/codegencommon.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 09ea2ee325cb11..c3608402d27ec2 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -3156,8 +3156,6 @@ void CodeGen::genHomeRegisterParams(regNumber initReg, bool* initRegStillZeroed) } #endif - // RISC-V specific logic for homing the stack part of a split struct - regMaskTP paramRegs = intRegState.rsCalleeRegArgMaskLiveIn | floatRegState.rsCalleeRegArgMaskLiveIn; if (compiler->opts.OptimizationDisabled()) {