From 794536f577e81d7a556dc79a54e0a12a97f8fee7 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 17 Dec 2024 14:49:12 +0100 Subject: [PATCH 1/9] Foo --- src/coreclr/jit/abi.cpp | 25 ++-- src/coreclr/jit/abi.h | 4 + src/coreclr/jit/codegen.h | 2 +- src/coreclr/jit/codegencommon.cpp | 117 ++++++++--------- src/coreclr/jit/compiler.cpp | 54 ++++++++ src/coreclr/jit/compiler.h | 30 ++++- src/coreclr/jit/liveness.cpp | 4 +- src/coreclr/jit/lower.cpp | 179 +++++++++++++++++++++++++ src/coreclr/jit/lower.h | 2 + src/coreclr/jit/lsra.h | 14 -- src/coreclr/jit/lsrabuild.cpp | 209 +++++------------------------- 11 files changed, 375 insertions(+), 265 deletions(-) diff --git a/src/coreclr/jit/abi.cpp b/src/coreclr/jit/abi.cpp index 04043c359246e..f31b14531aec8 100644 --- a/src/coreclr/jit/abi.cpp +++ b/src/coreclr/jit/abi.cpp @@ -465,15 +465,24 @@ void ABIPassingInformation::Dump() const } const ABIPassingSegment& seg = Segment(i); + seg.Dump(); + printf("\n"); + } +} - if (seg.IsPassedInRegister()) - { - printf("[%02u..%02u) reg %s\n", seg.Offset, seg.Offset + seg.Size, getRegName(seg.GetRegister())); - } - else - { - printf("[%02u..%02u) stack @ +%02u\n", seg.Offset, seg.Offset + seg.Size, seg.GetStackOffset()); - } +//----------------------------------------------------------------------------- +// Dump: +// Dump the ABIPassingSegment to stdout. +// +void ABIPassingSegment::Dump() const +{ + if (IsPassedInRegister()) + { + printf("[%02u..%02u) reg %s", Offset, Offset + Size, getRegName(GetRegister())); + } + else + { + printf("[%02u..%02u) stack @ +%02u", Offset, Offset + Size, GetStackOffset()); } } #endif diff --git a/src/coreclr/jit/abi.h b/src/coreclr/jit/abi.h index 66566f44db5fa..e1851ca8a6e92 100644 --- a/src/coreclr/jit/abi.h +++ b/src/coreclr/jit/abi.h @@ -38,6 +38,10 @@ class ABIPassingSegment static ABIPassingSegment InRegister(regNumber reg, unsigned offset, unsigned size); static ABIPassingSegment OnStack(unsigned stackOffset, unsigned offset, unsigned size); + +#ifdef DEBUG + void Dump() const; +#endif }; class ABIPassingSegmentIterator diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index 411a97129d4b7..8d6d2ef5684cc 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -260,7 +260,7 @@ class CodeGen final : public CodeGenInterface regMaskTP genGetParameterHomingTempRegisterCandidates(); var_types genParamStackType(LclVarDsc* dsc, const ABIPassingSegment& seg); - void genSpillOrAddRegisterParam(unsigned lclNum, class RegGraph* graph); + void genSpillOrAddRegisterParam(unsigned lclNum, const ABIPassingSegment& seg, class RegGraph* graph); void genSpillOrAddNonStandardRegisterParam(unsigned lclNum, regNumber sourceReg, class RegGraph* graph); void genEnregisterIncomingStackArgs(); #if defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 928fa6a6459b5..eb9a40aa09509 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -3239,10 +3239,11 @@ var_types CodeGen::genParamStackType(LclVarDsc* dsc, const ABIPassingSegment& se // to stack immediately, or by adding it to the register graph. // // Parameters: -// lclNum - Parameter local (or field of it) -// graph - The register graph to add to +// lclNum - Parameter local (or field of it) +// segment - Register segment to either spill or put in the register graph +// graph - The register graph to add to // -void CodeGen::genSpillOrAddRegisterParam(unsigned lclNum, RegGraph* graph) +void CodeGen::genSpillOrAddRegisterParam(unsigned lclNum, const ABIPassingSegment& segment, RegGraph* graph) { regMaskTP paramRegs = intRegState.rsCalleeRegArgMaskLiveIn | floatRegState.rsCalleeRegArgMaskLiveIn; LclVarDsc* varDsc = compiler->lvaGetDesc(lclNum); @@ -3250,72 +3251,59 @@ void CodeGen::genSpillOrAddRegisterParam(unsigned lclNum, RegGraph* graph) unsigned baseOffset = varDsc->lvIsStructField ? varDsc->lvFldOffset : 0; unsigned size = varDsc->lvExactSize(); - unsigned paramLclNum = varDsc->lvIsStructField ? varDsc->lvParentLcl : lclNum; - LclVarDsc* paramVarDsc = compiler->lvaGetDesc(paramLclNum); - const ABIPassingInformation& abiInfo = compiler->lvaGetParameterABIInfo(paramLclNum); - for (const ABIPassingSegment& seg : abiInfo.Segments()) + if (!segment.IsPassedInRegister() || ((paramRegs & genRegMask(segment.GetRegister())) == 0)) { - if (!seg.IsPassedInRegister() || ((paramRegs & genRegMask(seg.GetRegister())) == 0)) - { - continue; - } + return; + } - if (seg.Offset + seg.Size <= baseOffset) - { - continue; - } + if (varDsc->lvOnFrame && (!varDsc->lvIsInReg() || varDsc->lvLiveInOutOfHndlr)) + { + unsigned paramLclNum = varDsc->lvIsStructField ? varDsc->lvParentLcl : lclNum; + LclVarDsc* paramVarDsc = compiler->lvaGetDesc(paramLclNum); - if (baseOffset + size <= seg.Offset) + var_types storeType = genParamStackType(paramVarDsc, segment); + if ((varDsc->TypeGet() != TYP_STRUCT) && (genTypeSize(genActualType(varDsc)) < genTypeSize(storeType))) { - continue; + // Can happen for struct fields due to padding. + storeType = genActualType(varDsc); } - if (varDsc->lvOnFrame && (!varDsc->lvIsInReg() || varDsc->lvLiveInOutOfHndlr)) - { - var_types storeType = genParamStackType(paramVarDsc, seg); - if ((varDsc->TypeGet() != TYP_STRUCT) && (genTypeSize(genActualType(varDsc)) < genTypeSize(storeType))) - { - // Can happen for struct fields due to padding. - storeType = genActualType(varDsc); - } - - GetEmitter()->emitIns_S_R(ins_Store(storeType), emitActualTypeSize(storeType), seg.GetRegister(), lclNum, - seg.Offset - baseOffset); - } + GetEmitter()->emitIns_S_R(ins_Store(storeType), emitActualTypeSize(storeType), segment.GetRegister(), lclNum, + segment.Offset - baseOffset); + } - if (!varDsc->lvIsInReg()) - { - continue; - } + if (!varDsc->lvIsInReg()) + { + return; + } - var_types edgeType = genActualType(varDsc->GetRegisterType()); - // Some parameters can be passed in multiple registers but enregistered - // in a single one (e.g. SIMD types on arm64). In this case the edges - // we add here represent insertions of each element. - if (seg.Size < genTypeSize(edgeType)) - { - edgeType = seg.GetRegisterType(); - } + var_types edgeType = genActualType(varDsc->GetRegisterType()); + // Some parameters can be passed in multiple registers but enregistered + // in a single one (e.g. SIMD types on arm64). In this case the edges + // we add here represent insertions of each element. + if (segment.Size < genTypeSize(edgeType)) + { + edgeType = segment.GetRegisterType(); + } - RegNode* sourceReg = graph->GetOrAdd(seg.GetRegister()); - RegNode* destReg = graph->GetOrAdd(varDsc->GetRegNum()); + RegNode* sourceReg = graph->GetOrAdd(segment.GetRegister()); + RegNode* destReg = graph->GetOrAdd(varDsc->GetRegNum()); - if ((sourceReg != destReg) || (baseOffset != seg.Offset)) - { + if ((sourceReg != destReg) || (baseOffset != segment.Offset)) + { #ifdef TARGET_ARM - if (edgeType == TYP_DOUBLE) - { - assert(seg.Offset == baseOffset); - graph->AddEdge(sourceReg, destReg, TYP_FLOAT, 0); + if (edgeType == TYP_DOUBLE) + { + assert(segment.Offset == baseOffset); + graph->AddEdge(sourceReg, destReg, TYP_FLOAT, 0); - sourceReg = graph->GetOrAdd(REG_NEXT(sourceReg->reg)); - destReg = graph->GetOrAdd(REG_NEXT(destReg->reg)); - graph->AddEdge(sourceReg, destReg, TYP_FLOAT, 0); - continue; - } -#endif - graph->AddEdge(sourceReg, destReg, edgeType, seg.Offset - baseOffset); + sourceReg = graph->GetOrAdd(REG_NEXT(sourceReg->reg)); + destReg = graph->GetOrAdd(REG_NEXT(destReg->reg)); + graph->AddEdge(sourceReg, destReg, TYP_FLOAT, 0); + return; } +#endif + graph->AddEdge(sourceReg, destReg, edgeType, segment.Offset - baseOffset); } } @@ -3409,18 +3397,19 @@ void CodeGen::genHomeRegisterParams(regNumber initReg, bool* initRegStillZeroed) for (unsigned lclNum = 0; lclNum < compiler->info.compArgsCount; lclNum++) { LclVarDsc* lclDsc = compiler->lvaGetDesc(lclNum); + const ABIPassingInformation& abiInfo = compiler->lvaGetParameterABIInfo(lclNum); - if (compiler->lvaGetPromotionType(lclNum) == Compiler::PROMOTION_TYPE_INDEPENDENT) + for (const ABIPassingSegment& segment : abiInfo.Segments()) { - for (unsigned fld = 0; fld < lclDsc->lvFieldCnt; fld++) + if (!segment.IsPassedInRegister()) { - unsigned fieldLclNum = lclDsc->lvFieldLclStart + fld; - genSpillOrAddRegisterParam(fieldLclNum, &graph); + continue; } - } - else - { - genSpillOrAddRegisterParam(lclNum, &graph); + + const RegisterParameterLocalMapping* mapping = compiler->FindParameterRegisterLocalMappingByRegister(segment.GetRegister()); + + unsigned fieldLclNum = mapping != nullptr ? mapping->LclNum : lclNum; + genSpillOrAddRegisterParam(fieldLclNum, segment, &graph); } } diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index e2bae6d02fd67..42cfc101e934b 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -4221,6 +4221,60 @@ bool Compiler::compRsvdRegCheck(FrameLayoutState curState) } #endif // TARGET_ARMARCH || TARGET_RISCV64 +//------------------------------------------------------------------------ +// FindParameterRegisterLocalMappingByRegister: +// Try to find a mapping that maps a particular parameter register to an +// incoming defined local. +// +// Returns: +// The mapping, or nullptr if no mapping was found for this register. +// +const RegisterParameterLocalMapping* Compiler::FindParameterRegisterLocalMappingByRegister(regNumber reg) +{ + if (m_regParamLocalMappings == nullptr) + { + return nullptr; + } + + for (int i = 0; i < m_regParamLocalMappings->Height(); i++) + { + const RegisterParameterLocalMapping& mapping = m_regParamLocalMappings->BottomRef(i); + if (mapping.RegisterSegment->GetRegister() == reg) + { + return &mapping; + } + } + + return nullptr; +} + +//------------------------------------------------------------------------ +// FindParameterRegisterLocalMappingByLocal: +// Try to find a mapping that maps a particular local from an incoming +// parameter register. +// +// Returns: +// The mapping, or nullptr if no mapping was found for this local. +// +const RegisterParameterLocalMapping* Compiler::FindParameterRegisterLocalMappingByLocal(unsigned lclNum) +{ + if (m_regParamLocalMappings == nullptr) + { + return nullptr; + } + + for (int i = 0; i < m_regParamLocalMappings->Height(); i++) + { + const RegisterParameterLocalMapping& mapping = m_regParamLocalMappings->BottomRef(i); + if (mapping.LclNum == lclNum) + { + return &mapping; + } + } + + return nullptr; +} + //------------------------------------------------------------------------ // compGetTieringName: get a string describing tiered compilation settings // for this method diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 5e05ff24cf1b2..5363beb0ea8e9 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -522,6 +522,7 @@ class LclVarDsc unsigned char lvIsParam : 1; // is this a parameter? unsigned char lvIsRegArg : 1; // is this an argument that was passed by register? + unsigned char lvIsParamRegTarget : 1; // is this the target of a param reg to local mapping? unsigned char lvFramePointerBased : 1; // 0 = off of REG_SPBASE (e.g., ESP), 1 = off of REG_FPBASE (e.g., EBP) unsigned char lvOnFrame : 1; // (part of) the variable lives on the frame @@ -2580,6 +2581,24 @@ struct CloneTryInfo bool ScaleOriginalBlockProfile = false; }; +//------------------------------------------------------------------------ +// RegisterParameterLocalMapping: +// Contains mappings between a parameter register segment and a corresponding +// local. Used by the backend to know which locals are expected to contain +// which register parameters after the prolog. +// +struct RegisterParameterLocalMapping +{ + const ABIPassingSegment* RegisterSegment; + unsigned LclNum; + + RegisterParameterLocalMapping(const ABIPassingSegment* segment, unsigned lclNum) + : RegisterSegment(segment) + , LclNum(lclNum) + { + } +}; + /* XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX @@ -8291,8 +8310,14 @@ class Compiler bool rpMustCreateEBPFrame(INDEBUG(const char** wbReason)); private: - Lowering* m_pLowering; // Lowering; needed to Lower IR that's added or modified after Lowering. - LinearScanInterface* m_pLinearScan; // Linear Scan allocator + Lowering* m_pLowering = nullptr; // Lowering; needed to Lower IR that's added or modified after Lowering. + LinearScanInterface* m_pLinearScan = nullptr; // Linear Scan allocator + +public: + ArrayStack* m_regParamLocalMappings = nullptr; + + const RegisterParameterLocalMapping* FindParameterRegisterLocalMappingByRegister(regNumber reg); + const RegisterParameterLocalMapping* FindParameterRegisterLocalMappingByLocal(unsigned lclNum); /* XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX @@ -8307,7 +8332,6 @@ class Compiler XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX */ -public: // Get handles void eeGetCallInfo(CORINFO_RESOLVED_TOKEN* pResolvedToken, diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index b66da476b05c7..5452fb626cd56 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -1941,7 +1941,7 @@ void Compiler::fgInterBlockLocalVarLiveness() // liveness of such locals will bubble to the top (fgFirstBB) // in fgInterBlockLocalVarLiveness() - if (!varDsc->lvIsParam && VarSetOps::IsMember(this, fgFirstBB->bbLiveIn, varDsc->lvVarIndex) && + if (!varDsc->lvIsParam && !varDsc->lvIsParamRegTarget && VarSetOps::IsMember(this, fgFirstBB->bbLiveIn, varDsc->lvVarIndex) && (info.compInitMem || varTypeIsGC(varDsc->TypeGet())) && !fieldOfDependentlyPromotedStruct) { varDsc->lvMustInit = true; @@ -1962,7 +1962,7 @@ void Compiler::fgInterBlockLocalVarLiveness() if (isFinallyVar) { // Set lvMustInit only if we have a non-arg, GC pointer. - if (!varDsc->lvIsParam && varTypeIsGC(varDsc->TypeGet())) + if (!varDsc->lvIsParam && !varDsc->lvIsParamRegTarget && varTypeIsGC(varDsc->TypeGet())) { varDsc->lvMustInit = true; } diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 63a03c6097585..4575872f11192 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7850,6 +7850,11 @@ PhaseStatus Lowering::DoPhase() LowerBlock(block); } + if (comp->opts.OptimizationEnabled()) + { + MapParameterRegisterLocals(); + } + #ifdef DEBUG JITDUMP("Lower has completed modifying nodes.\n"); if (VERBOSE) @@ -7902,6 +7907,180 @@ PhaseStatus Lowering::DoPhase() return PhaseStatus::MODIFIED_EVERYTHING; } +//------------------------------------------------------------------------ +// Lowering::MapParameterRegisterLocals: +// Create mappings between parameter registers and locals corresponding to +// them. +// +void Lowering::MapParameterRegisterLocals() +{ + comp->m_regParamLocalMappings = new (comp, CMK_ABI) ArrayStack(comp->getAllocator(CMK_ABI)); + + // Create initial mappings for parameters. + for (unsigned lclNum = 0; lclNum < comp->info.compArgsCount; lclNum++) + { + LclVarDsc* lclDsc = comp->lvaGetDesc(lclNum); + const ABIPassingInformation& abiInfo = comp->lvaGetParameterABIInfo(lclNum); + + if (abiInfo.HasAnyStackSegment()) + { + continue; + } + + if (lclDsc->lvPromoted) + { + if (comp->lvaGetPromotionType(lclDsc) != Compiler::PROMOTION_TYPE_INDEPENDENT) + { + continue; + } + + for (const ABIPassingSegment& segment : abiInfo.Segments()) + { + unsigned fieldLclNum = comp->lvaGetFieldLocal(lclDsc, segment.Offset); + assert(fieldLclNum != BAD_VAR_NUM); + assert(segment.Size == lclDsc->lvExactSize()); + comp->m_regParamLocalMappings->Emplace(&segment, fieldLclNum); + + LclVarDsc* fieldLclDsc = comp->lvaGetDesc(fieldLclNum); + assert(!fieldLclDsc->lvIsParamRegTarget); + fieldLclDsc->lvIsParamRegTarget = true; + } + } + else + { + if (!abiInfo.HasExactlyOneRegisterSegment()) + { + continue; + } + + const ABIPassingSegment& segment = abiInfo.Segment(0); + if (!lclDsc->lvDoNotEnregister) + { + assert((segment.Size == lclDsc->lvExactSize()) || + ((lclDsc->lvExactSize() < segment.Size) && lclDsc->lvNormalizeOnLoad())); + comp->m_regParamLocalMappings->Emplace(&segment, lclNum); + assert(!lclDsc->lvIsParamRegTarget); + lclDsc->lvIsParamRegTarget = true; + } + } + } + + JitHashTable, bool> storedToLocals(comp->getAllocator(CMK_ABI)); + // Now look for optimization opportunities in the first block: places where + // we read fields out of struct parameters that can be mapped cleanly. This + // is frequently created by physical promotion. + for (GenTree* node : LIR::AsRange(comp->fgFirstBB)) + { + if (!node->OperIs(GT_STORE_LCL_VAR)) + { + continue; + } + + GenTreeLclVarCommon* storeLcl = node->AsLclVarCommon(); + LclVarDsc* destLclDsc = comp->lvaGetDesc(storeLcl); + + storedToLocals.Emplace(storeLcl->GetLclNum(), true); + + if (destLclDsc->lvIsParamRegTarget) + { + continue; + } + + if (destLclDsc->TypeGet() == TYP_STRUCT) + { + continue; + } + + GenTree* data = storeLcl->Data(); + if (!data->OperIs(GT_LCL_FLD)) + { + continue; + } + + GenTreeLclFld* dataFld = data->AsLclFld(); + if (dataFld->GetLclNum() >= comp->info.compArgsCount) + { + continue; + } + + // If the store comes with some form of widening/narrowing then we + // can't remap (it would require creating a new properly sized local). + if (dataFld->TypeGet() != storeLcl->TypeGet()) + { + continue; + } + + if (storedToLocals.Lookup(dataFld->GetLclNum())) + { + // Source is not necessarily the parameter anymore (could be if the + // store happened between the LCL_FLD and STORE_LCL_VAR, but we do + // not handle that here) + continue; + } + + const ABIPassingInformation& dataAbiInfo = comp->lvaGetParameterABIInfo(dataFld->GetLclNum()); + const ABIPassingSegment* regSegment = nullptr; + for (const ABIPassingSegment& segment : dataAbiInfo.Segments()) + { + if (!segment.IsPassedInRegister()) + { + continue; + } + + if ((segment.Offset != dataFld->GetLclOffs()) || (segment.Size != genTypeSize(dataFld))) + { + continue; + } + + bool hasMappingAlready = + (comp->FindParameterRegisterLocalMappingByRegister(segment.GetRegister()) != nullptr) || + (comp->FindParameterRegisterLocalMappingByLocal(storeLcl->GetLclNum()) != nullptr); + + if (hasMappingAlready) + { + continue; + } + + JITDUMP("Store from an unenregisterable parameter induces parameter register remapping. Store:\n"); + DISPTREERANGE(LIR::AsRange(comp->fgFirstBB), node); + + comp->m_regParamLocalMappings->Emplace(&segment, storeLcl->GetLclNum()); + destLclDsc->lvIsParamRegTarget = true; + node->gtBashToNOP(); + dataFld->SetUnusedValue(); + + JITDUMP("New mapping: "); + DBEXEC(VERBOSE, segment.Dump()); + JITDUMP(" -> V%02u\n", storeLcl->GetLclNum()); + + GenTree* regParamValue = comp->gtNewLclvNode(storeLcl->GetLclNum(), dataFld->TypeGet()); + GenTree* storeField = comp->gtNewStoreLclFldNode(dataFld->GetLclNum(), dataFld->TypeGet(), segment.Offset, regParamValue); + + // Store actual parameter local from new reg local + LIR::AsRange(comp->fgFirstBB).InsertAtBeginning(LIR::SeqTree(comp, storeField)); + LowerNode(regParamValue); + LowerNode(storeField); + + JITDUMP("Parameter spill:\n"); + DISPTREERANGE(LIR::AsRange(comp->fgFirstBB), storeField); + } + } + +#ifdef DEBUG + if (comp->verbose) + { + printf("%d parameter register to local mappings\n", comp->m_regParamLocalMappings->Height()); + for (int i = 0; i < comp->m_regParamLocalMappings->Height(); i++) + { + const RegisterParameterLocalMapping& mapping = comp->m_regParamLocalMappings->BottomRef(i); + printf(" "); + mapping.RegisterSegment->Dump(); + printf(" -> V%02u\n", mapping.LclNum); + } + } +#endif +} + #ifdef DEBUG //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index 9aee0fd99a120..659870c844cd7 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -132,6 +132,8 @@ class Lowering final : public Phase static bool CheckBlock(Compiler* compiler, BasicBlock* block); #endif // DEBUG + void MapParameterRegisterLocals(); + void LowerBlock(BasicBlock* block); GenTree* LowerNode(GenTree* node); diff --git a/src/coreclr/jit/lsra.h b/src/coreclr/jit/lsra.h index 9312eb46a52b2..d2b35184b81e5 100644 --- a/src/coreclr/jit/lsra.h +++ b/src/coreclr/jit/lsra.h @@ -1038,20 +1038,6 @@ class LinearScan : public LinearScanInterface Interval* lclVarInterval, LsraLocation currentLoc, GenTree* node, bool isUse, unsigned multiRegIdx); #endif // FEATURE_PARTIAL_SIMD_CALLEE_SAVE -#if defined(UNIX_AMD64_ABI) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64) - // For AMD64 on SystemV machines. This method - // is called as replacement for raUpdateRegStateForArg - // that is used on Windows. On System V systems a struct can be passed - // partially using registers from the 2 register files. - // - // For LoongArch64's ABI, a struct can be passed - // partially using registers from the 2 register files. - void UpdateRegStateForStructArg(LclVarDsc* argDsc); -#endif // defined(UNIX_AMD64_ABI) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64) - - // Update reg state for an incoming register argument - void updateRegStateForArg(LclVarDsc* argDsc); - inline bool isCandidateLocalRef(GenTree* tree) { if (tree->IsLocal()) diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 56bba3469eb27..55b37e71ee95b 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -2029,7 +2029,7 @@ void LinearScan::insertZeroInitRefPositions() while (iter.NextElem(&varIndex)) { LclVarDsc* varDsc = compiler->lvaGetDescByTrackedIndex(varIndex); - if (!varDsc->lvIsParam && isCandidateVar(varDsc)) + if (!varDsc->lvIsParam && !varDsc->lvIsParamRegTarget && isCandidateVar(varDsc)) { JITDUMP("V%02u was live in to first block:", compiler->lvaTrackedIndexToLclNum(varIndex)); Interval* interval = getIntervalForLocalVar(varIndex); @@ -2094,110 +2094,6 @@ void LinearScan::insertZeroInitRefPositions() } } -#if defined(UNIX_AMD64_ABI) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64) -//------------------------------------------------------------------------ -// UpdateRegStateForStructArg: -// Sets the register state for an argument of type STRUCT. -// This is shared between with AMD64's SystemV systems and LoongArch64-ABI. -// -// Arguments: -// argDsc - the LclVarDsc for the argument of interest -// -// Notes: -// See Compiler::raUpdateRegStateForArg(RegState *regState, LclVarDsc *argDsc) in regalloc.cpp -// for how state for argument is updated for unix non-structs and Windows AMD64 structs. -// -void LinearScan::UpdateRegStateForStructArg(LclVarDsc* argDsc) -{ - assert(varTypeIsStruct(argDsc)); - RegState* intRegState = &compiler->codeGen->intRegState; - RegState* floatRegState = &compiler->codeGen->floatRegState; - - if ((argDsc->GetArgReg() != REG_STK) && (argDsc->GetArgReg() != REG_NA)) - { - if ((genRegMask(argDsc->GetArgReg()) & RBM_ALLFLOAT) != RBM_NONE) - { - assert((genRegMask(argDsc->GetArgReg()) & RBM_FLTARG_REGS) != RBM_NONE); - floatRegState->rsCalleeRegArgMaskLiveIn.AddRegNumInMask(argDsc->GetArgReg()); - } - else - { - assert((genRegMask(argDsc->GetArgReg()) & fullIntArgRegMask(compiler->info.compCallConv)) != RBM_NONE); - intRegState->rsCalleeRegArgMaskLiveIn.AddRegNumInMask(argDsc->GetArgReg()); - } - } - - if ((argDsc->GetOtherArgReg() != REG_STK) && (argDsc->GetOtherArgReg() != REG_NA)) - { - if (genRegMask(argDsc->GetOtherArgReg()) & (RBM_ALLFLOAT)) - { - assert(genRegMask(argDsc->GetOtherArgReg()) & (RBM_FLTARG_REGS)); - floatRegState->rsCalleeRegArgMaskLiveIn.AddRegNumInMask(argDsc->GetOtherArgReg()); - } - else - { - assert((genRegMask(argDsc->GetOtherArgReg()) & fullIntArgRegMask(compiler->info.compCallConv)) != RBM_NONE); - intRegState->rsCalleeRegArgMaskLiveIn.AddRegNumInMask(argDsc->GetOtherArgReg()); - } - } -} - -#endif // defined(UNIX_AMD64_ABI) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64) - -//------------------------------------------------------------------------ -// updateRegStateForArg: Updates rsCalleeRegArgMaskLiveIn for the appropriate -// regState (either compiler->intRegState or compiler->floatRegState), -// with the lvArgReg on "argDsc" -// -// Arguments: -// argDsc - the argument for which the state is to be updated. -// -// Return Value: None -// -// Assumptions: -// The argument is live on entry to the function -// (or is untracked and therefore assumed live) -// -void LinearScan::updateRegStateForArg(LclVarDsc* argDsc) -{ -#if defined(UNIX_AMD64_ABI) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64) - // For SystemV-AMD64 and LoongArch64 calls the argDsc - // can have 2 registers (for structs.). Handle them here. - if (varTypeIsStruct(argDsc)) - { - UpdateRegStateForStructArg(argDsc); - } - else -#endif // defined(UNIX_AMD64_ABI) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64) - { - RegState* intRegState = &compiler->codeGen->intRegState; - RegState* floatRegState = &compiler->codeGen->floatRegState; - bool isFloat = emitter::isFloatReg(argDsc->GetArgReg()); - - if (argDsc->lvIsHfaRegArg()) - { - isFloat = true; - } - - if (isFloat) - { - JITDUMP("Float arg V%02u in reg %s\n", compiler->lvaGetLclNum(argDsc), getRegName(argDsc->GetArgReg())); - compiler->raUpdateRegStateForArg(floatRegState, argDsc); - } - else - { - JITDUMP("Int arg V%02u in reg %s\n", compiler->lvaGetLclNum(argDsc), getRegName(argDsc->GetArgReg())); -#if FEATURE_MULTIREG_ARGS - if (argDsc->GetOtherArgReg() != REG_NA) - { - JITDUMP("(second half) in reg %s\n", getRegName(argDsc->GetOtherArgReg())); - } -#endif // FEATURE_MULTIREG_ARGS - compiler->raUpdateRegStateForArg(intRegState, argDsc); - } - } -} - template void LinearScan::buildIntervals(); template void LinearScan::buildIntervals(); @@ -2279,36 +2175,36 @@ void LinearScan::buildIntervals() regsInUseThisLocation = RBM_NONE; regsInUseNextLocation = RBM_NONE; -#ifdef SWIFT_SUPPORT - if (compiler->info.compCallConv == CorInfoCallConvExtension::Swift) + for (unsigned lclNum = 0; lclNum < compiler->info.compArgsCount; lclNum++) { - for (unsigned lclNum = 0; lclNum < compiler->info.compArgsCount; lclNum++) + const ABIPassingInformation& abiInfo = compiler->lvaGetParameterABIInfo(lclNum); + for (const ABIPassingSegment& seg : abiInfo.Segments()) { - LclVarDsc* argDsc = compiler->lvaGetDesc(lclNum); - - if ((argDsc->lvRefCnt() == 0) && !compiler->opts.compDbgCode) + if (!seg.IsPassedInRegister()) { continue; } - const ABIPassingInformation& abiInfo = compiler->lvaGetParameterABIInfo(lclNum); - for (const ABIPassingSegment& seg : abiInfo.Segments()) + const RegisterParameterLocalMapping* mapping = compiler->FindParameterRegisterLocalMappingByRegister(seg.GetRegister()); + + LclVarDsc* argDsc = compiler->lvaGetDesc(mapping != nullptr ? mapping->LclNum : lclNum); + if (!compiler->compJmpOpUsed && (argDsc->lvRefCnt() == 0) && !compiler->opts.compDbgCode) { - if (seg.IsPassedInRegister()) - { - RegState* regState = genIsValidFloatReg(seg.GetRegister()) ? floatRegState : intRegState; - regState->rsCalleeRegArgMaskLiveIn |= seg.GetRegisterMask(); - } + // Not live + continue; } + + RegState* regState = genIsValidFloatReg(seg.GetRegister()) ? floatRegState : intRegState; + regState->rsCalleeRegArgMaskLiveIn |= seg.GetRegisterMask(); } } -#endif for (unsigned int varIndex = 0; varIndex < compiler->lvaTrackedCount; varIndex++) { - LclVarDsc* argDsc = compiler->lvaGetDescByTrackedIndex(varIndex); + unsigned lclNum = compiler->lvaTrackedIndexToLclNum(varIndex); + LclVarDsc* lclDsc = compiler->lvaGetDesc(lclNum); - if (!argDsc->lvIsParam) + if (!isCandidateVar(lclDsc)) { continue; } @@ -2319,70 +2215,37 @@ void LinearScan::buildIntervals() // Use lvRefCnt instead of checking bbLiveIn because if it's volatile we // won't have done dataflow on it, but it needs to be marked as live-in so // it will get saved in the prolog. - if (!compiler->compJmpOpUsed && argDsc->lvRefCnt() == 0 && !compiler->opts.compDbgCode) + if (!compiler->compJmpOpUsed && (lclDsc->lvRefCnt() == 0) && !compiler->opts.compDbgCode) { continue; } - if (argDsc->lvIsRegArg) - { - updateRegStateForArg(argDsc); - } - - if (isCandidateVar(argDsc)) - { - buildInitialParamDef(argDsc, argDsc->lvIsRegArg ? argDsc->GetArgReg() : REG_NA); - } - else if (argDsc->lvPromoted) + regNumber paramReg = REG_NA; + if (lclDsc->lvIsParamRegTarget) { - for (unsigned fieldVarNum = argDsc->lvFieldLclStart; - fieldVarNum < argDsc->lvFieldLclStart + argDsc->lvFieldCnt; ++fieldVarNum) - { - const LclVarDsc* fieldVarDsc = compiler->lvaGetDesc(fieldVarNum); - if (fieldVarDsc->lvLRACandidate) - { - assert(fieldVarDsc->lvTracked); - buildInitialParamDef(fieldVarDsc, REG_NA); - } - } + const RegisterParameterLocalMapping* mapping = compiler->FindParameterRegisterLocalMappingByLocal(lclNum); + assert(mapping != nullptr); + paramReg = mapping->RegisterSegment->GetRegister(); } - else + else if (lclDsc->lvIsParam) { - // We can overwrite the register (i.e. codegen saves it on entry) - assert(argDsc->lvRefCnt() == 0 || !argDsc->lvIsRegArg || argDsc->lvDoNotEnregister || - !argDsc->lvLRACandidate || (varTypeIsFloating(argDsc->TypeGet()) && compiler->opts.compDbgCode)); - } - } - - // Now set up the reg state for the non-tracked args - // (We do this here because we want to generate the ParamDef RefPositions in tracked - // order, so that loop doesn't hit the non-tracked args) - - for (unsigned argNum = 0; argNum < compiler->info.compArgsCount; argNum++) - { - LclVarDsc* argDsc = compiler->lvaGetDesc(argNum); + // This is a parameter that is a register candidate but not a + // parameter register target. It must be a stack parameter -- + // lowering ensures all potentially enregisterable register + // parameters are marked as lvIsParamRegTarget. +#ifdef DEBUG + unsigned abiLclNum = lclDsc->lvIsStructField ? lclDsc->lvParentLcl : lclNum; + assert(!compiler->lvaGetParameterABIInfo(abiLclNum).HasAnyRegisterSegment()); +#endif - if (argDsc->lvPromoted) - { - for (unsigned fieldVarNum = argDsc->lvFieldLclStart; - fieldVarNum < argDsc->lvFieldLclStart + argDsc->lvFieldCnt; ++fieldVarNum) - { - LclVarDsc* fieldVarDsc = compiler->lvaGetDesc(fieldVarNum); - noway_assert(fieldVarDsc->lvIsParam); - if (!fieldVarDsc->lvTracked && fieldVarDsc->lvIsRegArg) - { - updateRegStateForArg(fieldVarDsc); - } - } + // Fall through with paramReg = REG_NA } else { - noway_assert(argDsc->lvIsParam); - if (!argDsc->lvTracked && argDsc->lvIsRegArg) - { - updateRegStateForArg(argDsc); - } + continue; } + + buildInitialParamDef(lclDsc, paramReg); } // If there is a secret stub param, it is also live in From db15f945c0b006cc55cadfd05330e0c5f8710f50 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 17 Dec 2024 16:06:19 +0100 Subject: [PATCH 2/9] Remove optimization for now --- src/coreclr/jit/lower.cpp | 101 -------------------------------------- 1 file changed, 101 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 4575872f11192..f47d28500cc90 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7965,107 +7965,6 @@ void Lowering::MapParameterRegisterLocals() } } - JitHashTable, bool> storedToLocals(comp->getAllocator(CMK_ABI)); - // Now look for optimization opportunities in the first block: places where - // we read fields out of struct parameters that can be mapped cleanly. This - // is frequently created by physical promotion. - for (GenTree* node : LIR::AsRange(comp->fgFirstBB)) - { - if (!node->OperIs(GT_STORE_LCL_VAR)) - { - continue; - } - - GenTreeLclVarCommon* storeLcl = node->AsLclVarCommon(); - LclVarDsc* destLclDsc = comp->lvaGetDesc(storeLcl); - - storedToLocals.Emplace(storeLcl->GetLclNum(), true); - - if (destLclDsc->lvIsParamRegTarget) - { - continue; - } - - if (destLclDsc->TypeGet() == TYP_STRUCT) - { - continue; - } - - GenTree* data = storeLcl->Data(); - if (!data->OperIs(GT_LCL_FLD)) - { - continue; - } - - GenTreeLclFld* dataFld = data->AsLclFld(); - if (dataFld->GetLclNum() >= comp->info.compArgsCount) - { - continue; - } - - // If the store comes with some form of widening/narrowing then we - // can't remap (it would require creating a new properly sized local). - if (dataFld->TypeGet() != storeLcl->TypeGet()) - { - continue; - } - - if (storedToLocals.Lookup(dataFld->GetLclNum())) - { - // Source is not necessarily the parameter anymore (could be if the - // store happened between the LCL_FLD and STORE_LCL_VAR, but we do - // not handle that here) - continue; - } - - const ABIPassingInformation& dataAbiInfo = comp->lvaGetParameterABIInfo(dataFld->GetLclNum()); - const ABIPassingSegment* regSegment = nullptr; - for (const ABIPassingSegment& segment : dataAbiInfo.Segments()) - { - if (!segment.IsPassedInRegister()) - { - continue; - } - - if ((segment.Offset != dataFld->GetLclOffs()) || (segment.Size != genTypeSize(dataFld))) - { - continue; - } - - bool hasMappingAlready = - (comp->FindParameterRegisterLocalMappingByRegister(segment.GetRegister()) != nullptr) || - (comp->FindParameterRegisterLocalMappingByLocal(storeLcl->GetLclNum()) != nullptr); - - if (hasMappingAlready) - { - continue; - } - - JITDUMP("Store from an unenregisterable parameter induces parameter register remapping. Store:\n"); - DISPTREERANGE(LIR::AsRange(comp->fgFirstBB), node); - - comp->m_regParamLocalMappings->Emplace(&segment, storeLcl->GetLclNum()); - destLclDsc->lvIsParamRegTarget = true; - node->gtBashToNOP(); - dataFld->SetUnusedValue(); - - JITDUMP("New mapping: "); - DBEXEC(VERBOSE, segment.Dump()); - JITDUMP(" -> V%02u\n", storeLcl->GetLclNum()); - - GenTree* regParamValue = comp->gtNewLclvNode(storeLcl->GetLclNum(), dataFld->TypeGet()); - GenTree* storeField = comp->gtNewStoreLclFldNode(dataFld->GetLclNum(), dataFld->TypeGet(), segment.Offset, regParamValue); - - // Store actual parameter local from new reg local - LIR::AsRange(comp->fgFirstBB).InsertAtBeginning(LIR::SeqTree(comp, storeField)); - LowerNode(regParamValue); - LowerNode(storeField); - - JITDUMP("Parameter spill:\n"); - DISPTREERANGE(LIR::AsRange(comp->fgFirstBB), storeField); - } - } - #ifdef DEBUG if (comp->verbose) { From 8e8c913b9e2cd004992d196c9fa99c8d293e303f Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 17 Dec 2024 16:16:59 +0100 Subject: [PATCH 3/9] Add an lvTracked check, remove dead code --- src/coreclr/jit/compiler.h | 2 - src/coreclr/jit/lsrabuild.cpp | 2 +- src/coreclr/jit/regalloc.cpp | 74 ----------------------------------- 3 files changed, 1 insertion(+), 77 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 5363beb0ea8e9..aefc67d8ddc42 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -8277,8 +8277,6 @@ class Compiler */ public: - regNumber raUpdateRegStateForArg(RegState* regState, LclVarDsc* argDsc); - void raMarkStkVars(); #if FEATURE_PARTIAL_SIMD_CALLEE_SAVE diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 55b37e71ee95b..3f99bdb2c21aa 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -2188,7 +2188,7 @@ void LinearScan::buildIntervals() const RegisterParameterLocalMapping* mapping = compiler->FindParameterRegisterLocalMappingByRegister(seg.GetRegister()); LclVarDsc* argDsc = compiler->lvaGetDesc(mapping != nullptr ? mapping->LclNum : lclNum); - if (!compiler->compJmpOpUsed && (argDsc->lvRefCnt() == 0) && !compiler->opts.compDbgCode) + if (argDsc->lvTracked && !compiler->compJmpOpUsed && (argDsc->lvRefCnt() == 0) && !compiler->opts.compDbgCode) { // Not live continue; diff --git a/src/coreclr/jit/regalloc.cpp b/src/coreclr/jit/regalloc.cpp index ba07086a63c21..1de4dd8bd6699 100644 --- a/src/coreclr/jit/regalloc.cpp +++ b/src/coreclr/jit/regalloc.cpp @@ -95,80 +95,6 @@ bool Compiler::shouldDoubleAlign( } #endif // DOUBLE_ALIGN -// The code to set the regState for each arg is outlined for shared use -// by linear scan. (It is not shared for System V AMD64 platform.) -regNumber Compiler::raUpdateRegStateForArg(RegState* regState, LclVarDsc* argDsc) -{ - regNumber inArgReg = argDsc->GetArgReg(); - regMaskTP inArgMask = genRegMask(inArgReg); - - if (regState->rsIsFloat) - { - assert((inArgMask & RBM_FLTARG_REGS) != RBM_NONE); - } - else - { - assert((inArgMask & fullIntArgRegMask(info.compCallConv)) != RBM_NONE); - } - - regState->rsCalleeRegArgMaskLiveIn |= inArgMask; - -#ifdef TARGET_ARM - if (argDsc->lvType == TYP_DOUBLE) - { - if (info.compIsVarArgs || opts.compUseSoftFP) - { - assert((inArgReg == REG_R0) || (inArgReg == REG_R2)); - assert(!regState->rsIsFloat); - } - else - { - assert(regState->rsIsFloat); - assert(emitter::isDoubleReg(inArgReg)); - } - regState->rsCalleeRegArgMaskLiveIn |= genRegMask((regNumber)(inArgReg + 1)); - } - else if (argDsc->lvType == TYP_LONG) - { - assert((inArgReg == REG_R0) || (inArgReg == REG_R2)); - assert(!regState->rsIsFloat); - regState->rsCalleeRegArgMaskLiveIn |= genRegMask((regNumber)(inArgReg + 1)); - } -#endif // TARGET_ARM - -#if FEATURE_MULTIREG_ARGS - if (varTypeIsStruct(argDsc->lvType)) - { - if (argDsc->lvIsHfaRegArg()) - { - assert(regState->rsIsFloat); - unsigned cSlots = argDsc->lvHfaSlots(); - for (unsigned i = 1; i < cSlots; i++) - { - assert(inArgReg + i <= LAST_FP_ARGREG); - regState->rsCalleeRegArgMaskLiveIn |= genRegMask(static_cast(inArgReg + i)); - } - } - else - { - assert(!regState->rsIsFloat); - unsigned cSlots = argDsc->lvSize() / TARGET_POINTER_SIZE; - for (unsigned i = 1; i < cSlots; i++) - { - regNumber nextArgReg = (regNumber)(inArgReg + i); - if (nextArgReg > REG_ARG_LAST) - { - break; - } - regState->rsCalleeRegArgMaskLiveIn |= genRegMask(nextArgReg); - } - } - } -#endif // FEATURE_MULTIREG_ARGS - - return inArgReg; -} - //------------------------------------------------------------------------ // rpMustCreateEBPFrame: // Returns true when we must create an EBP frame From e504b4bf259554ea4e683a537ec51eda82110d56 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 18 Dec 2024 00:58:02 +0100 Subject: [PATCH 4/9] Support insertions for now --- src/coreclr/jit/compiler.cpp | 8 ++++-- src/coreclr/jit/compiler.h | 14 +++++++-- src/coreclr/jit/lower.cpp | 53 +++++++++++++++-------------------- src/coreclr/jit/lsrabuild.cpp | 33 +++++++++++++++------- 4 files changed, 63 insertions(+), 45 deletions(-) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 42cfc101e934b..b73cab83bfb1f 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -4253,10 +4253,14 @@ const RegisterParameterLocalMapping* Compiler::FindParameterRegisterLocalMapping // Try to find a mapping that maps a particular local from an incoming // parameter register. // +// Parameters: +// lclNum - The local to find a mapping for +// offset - The offset that the mapping maps to in the local +// // Returns: // The mapping, or nullptr if no mapping was found for this local. // -const RegisterParameterLocalMapping* Compiler::FindParameterRegisterLocalMappingByLocal(unsigned lclNum) +const RegisterParameterLocalMapping* Compiler::FindParameterRegisterLocalMappingByLocal(unsigned lclNum, unsigned offset) { if (m_regParamLocalMappings == nullptr) { @@ -4266,7 +4270,7 @@ const RegisterParameterLocalMapping* Compiler::FindParameterRegisterLocalMapping for (int i = 0; i < m_regParamLocalMappings->Height(); i++) { const RegisterParameterLocalMapping& mapping = m_regParamLocalMappings->BottomRef(i); - if (mapping.LclNum == lclNum) + if ((mapping.LclNum == lclNum) && (mapping.Offset == offset)) { return &mapping; } diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index aefc67d8ddc42..67067098307d9 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2591,10 +2591,18 @@ struct RegisterParameterLocalMapping { const ABIPassingSegment* RegisterSegment; unsigned LclNum; - - RegisterParameterLocalMapping(const ABIPassingSegment* segment, unsigned lclNum) + // Offset at which the register is inserted into the local. Used e.g. for + // HFAs on arm64 that might have been promoted as a single local (e.g. + // System.Numerics.Plane is passed in 3 float regs but enregistered as + // TYP_SIMD12). + // SysV 64 also see similar situations e.g. Vector3 being passed in + // xmm0[0..8), xmm1[8..12), but enregistered as one register. + unsigned Offset; + + RegisterParameterLocalMapping(const ABIPassingSegment* segment, unsigned lclNum, unsigned offset) : RegisterSegment(segment) , LclNum(lclNum) + , Offset(offset) { } }; @@ -8315,7 +8323,7 @@ class Compiler ArrayStack* m_regParamLocalMappings = nullptr; const RegisterParameterLocalMapping* FindParameterRegisterLocalMappingByRegister(regNumber reg); - const RegisterParameterLocalMapping* FindParameterRegisterLocalMappingByLocal(unsigned lclNum); + const RegisterParameterLocalMapping* FindParameterRegisterLocalMappingByLocal(unsigned lclNum, unsigned offset); /* XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index f47d28500cc90..a7ce817092368 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7916,7 +7916,7 @@ void Lowering::MapParameterRegisterLocals() { comp->m_regParamLocalMappings = new (comp, CMK_ABI) ArrayStack(comp->getAllocator(CMK_ABI)); - // Create initial mappings for parameters. + // Create initial mappings for promotions. for (unsigned lclNum = 0; lclNum < comp->info.compArgsCount; lclNum++) { LclVarDsc* lclDsc = comp->lvaGetDesc(lclNum); @@ -7927,41 +7927,34 @@ void Lowering::MapParameterRegisterLocals() continue; } - if (lclDsc->lvPromoted) + if (comp->lvaGetPromotionType(lclDsc) != Compiler::PROMOTION_TYPE_INDEPENDENT) { - if (comp->lvaGetPromotionType(lclDsc) != Compiler::PROMOTION_TYPE_INDEPENDENT) - { - continue; - } - - for (const ABIPassingSegment& segment : abiInfo.Segments()) - { - unsigned fieldLclNum = comp->lvaGetFieldLocal(lclDsc, segment.Offset); - assert(fieldLclNum != BAD_VAR_NUM); - assert(segment.Size == lclDsc->lvExactSize()); - comp->m_regParamLocalMappings->Emplace(&segment, fieldLclNum); - - LclVarDsc* fieldLclDsc = comp->lvaGetDesc(fieldLclNum); - assert(!fieldLclDsc->lvIsParamRegTarget); - fieldLclDsc->lvIsParamRegTarget = true; - } + continue; } - else + + for (int i = 0; i < lclDsc->lvFieldCnt; i++) { - if (!abiInfo.HasExactlyOneRegisterSegment()) - { - continue; - } + unsigned fieldLclNum = lclDsc->lvFieldLclStart + i; + LclVarDsc* fieldDsc = comp->lvaGetDesc(fieldLclNum); - const ABIPassingSegment& segment = abiInfo.Segment(0); - if (!lclDsc->lvDoNotEnregister) + for (const ABIPassingSegment& segment : abiInfo.Segments()) { - assert((segment.Size == lclDsc->lvExactSize()) || - ((lclDsc->lvExactSize() < segment.Size) && lclDsc->lvNormalizeOnLoad())); - comp->m_regParamLocalMappings->Emplace(&segment, lclNum); - assert(!lclDsc->lvIsParamRegTarget); - lclDsc->lvIsParamRegTarget = true; + if (segment.Offset + segment.Size <= fieldDsc->lvFldOffset) + { + continue; + } + + if (fieldDsc->lvFldOffset + fieldDsc->lvExactSize() <= segment.Offset) + { + continue; + } + + comp->m_regParamLocalMappings->Emplace(&segment, fieldLclNum, segment.Offset - fieldDsc->lvFldOffset); } + + LclVarDsc* fieldLclDsc = comp->lvaGetDesc(fieldLclNum); + assert(!fieldLclDsc->lvIsParamRegTarget); + fieldLclDsc->lvIsParamRegTarget = true; } } diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 3f99bdb2c21aa..f542f245f4b3b 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -2223,22 +2223,35 @@ void LinearScan::buildIntervals() regNumber paramReg = REG_NA; if (lclDsc->lvIsParamRegTarget) { - const RegisterParameterLocalMapping* mapping = compiler->FindParameterRegisterLocalMappingByLocal(lclNum); + const RegisterParameterLocalMapping* mapping = compiler->FindParameterRegisterLocalMappingByLocal(lclNum, 0); assert(mapping != nullptr); paramReg = mapping->RegisterSegment->GetRegister(); } else if (lclDsc->lvIsParam) { - // This is a parameter that is a register candidate but not a - // parameter register target. It must be a stack parameter -- - // lowering ensures all potentially enregisterable register - // parameters are marked as lvIsParamRegTarget. -#ifdef DEBUG - unsigned abiLclNum = lclDsc->lvIsStructField ? lclDsc->lvParentLcl : lclNum; - assert(!compiler->lvaGetParameterABIInfo(abiLclNum).HasAnyRegisterSegment()); -#endif + if (lclDsc->lvIsStructField) + { + // All promoted fields should be assigned via the + // lvIsParamRegTarget mechanism, so this must be a stack + // argument. + assert(!compiler->lvaGetParameterABIInfo(lclDsc->lvParentLcl).HasAnyRegisterSegment()); - // Fall through with paramReg = REG_NA + // Fall through with paramReg == REG_NA + } + else + { + // Enregisterable parameter, may or may not be a stack arg. + // Prefer the first register if there is one. + const ABIPassingInformation& abiInfo = compiler->lvaGetParameterABIInfo(lclNum); + for (const ABIPassingSegment& seg : abiInfo.Segments()) + { + if (seg.IsPassedInRegister()) + { + paramReg = seg.GetRegister(); + break; + } + } + } } else { From bf196bd0ca57d1ec65c5557b51eeec8169681d36 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 18 Dec 2024 00:58:33 +0100 Subject: [PATCH 5/9] Run jit-format --- src/coreclr/jit/codegencommon.cpp | 9 +++++---- src/coreclr/jit/compiler.cpp | 3 ++- src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/liveness.cpp | 3 ++- src/coreclr/jit/lower.cpp | 9 +++++---- src/coreclr/jit/lsrabuild.cpp | 11 +++++++---- 6 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index eb9a40aa09509..fa61ae5087e31 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -3258,8 +3258,8 @@ void CodeGen::genSpillOrAddRegisterParam(unsigned lclNum, const ABIPassingSegmen if (varDsc->lvOnFrame && (!varDsc->lvIsInReg() || varDsc->lvLiveInOutOfHndlr)) { - unsigned paramLclNum = varDsc->lvIsStructField ? varDsc->lvParentLcl : lclNum; - LclVarDsc* paramVarDsc = compiler->lvaGetDesc(paramLclNum); + unsigned paramLclNum = varDsc->lvIsStructField ? varDsc->lvParentLcl : lclNum; + LclVarDsc* paramVarDsc = compiler->lvaGetDesc(paramLclNum); var_types storeType = genParamStackType(paramVarDsc, segment); if ((varDsc->TypeGet() != TYP_STRUCT) && (genTypeSize(genActualType(varDsc)) < genTypeSize(storeType))) @@ -3396,7 +3396,7 @@ void CodeGen::genHomeRegisterParams(regNumber initReg, bool* initRegStillZeroed) for (unsigned lclNum = 0; lclNum < compiler->info.compArgsCount; lclNum++) { - LclVarDsc* lclDsc = compiler->lvaGetDesc(lclNum); + LclVarDsc* lclDsc = compiler->lvaGetDesc(lclNum); const ABIPassingInformation& abiInfo = compiler->lvaGetParameterABIInfo(lclNum); for (const ABIPassingSegment& segment : abiInfo.Segments()) @@ -3406,7 +3406,8 @@ void CodeGen::genHomeRegisterParams(regNumber initReg, bool* initRegStillZeroed) continue; } - const RegisterParameterLocalMapping* mapping = compiler->FindParameterRegisterLocalMappingByRegister(segment.GetRegister()); + const RegisterParameterLocalMapping* mapping = + compiler->FindParameterRegisterLocalMappingByRegister(segment.GetRegister()); unsigned fieldLclNum = mapping != nullptr ? mapping->LclNum : lclNum; genSpillOrAddRegisterParam(fieldLclNum, segment, &graph); diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index b73cab83bfb1f..e1aba64032cec 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -4260,7 +4260,8 @@ const RegisterParameterLocalMapping* Compiler::FindParameterRegisterLocalMapping // Returns: // The mapping, or nullptr if no mapping was found for this local. // -const RegisterParameterLocalMapping* Compiler::FindParameterRegisterLocalMappingByLocal(unsigned lclNum, unsigned offset) +const RegisterParameterLocalMapping* Compiler::FindParameterRegisterLocalMappingByLocal(unsigned lclNum, + unsigned offset) { if (m_regParamLocalMappings == nullptr) { diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 67067098307d9..84cc274c15b6e 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -8316,7 +8316,7 @@ class Compiler bool rpMustCreateEBPFrame(INDEBUG(const char** wbReason)); private: - Lowering* m_pLowering = nullptr; // Lowering; needed to Lower IR that's added or modified after Lowering. + Lowering* m_pLowering = nullptr; // Lowering; needed to Lower IR that's added or modified after Lowering. LinearScanInterface* m_pLinearScan = nullptr; // Linear Scan allocator public: diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index 5452fb626cd56..726b4a5e3b276 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -1941,7 +1941,8 @@ void Compiler::fgInterBlockLocalVarLiveness() // liveness of such locals will bubble to the top (fgFirstBB) // in fgInterBlockLocalVarLiveness() - if (!varDsc->lvIsParam && !varDsc->lvIsParamRegTarget && VarSetOps::IsMember(this, fgFirstBB->bbLiveIn, varDsc->lvVarIndex) && + if (!varDsc->lvIsParam && !varDsc->lvIsParamRegTarget && + VarSetOps::IsMember(this, fgFirstBB->bbLiveIn, varDsc->lvVarIndex) && (info.compInitMem || varTypeIsGC(varDsc->TypeGet())) && !fieldOfDependentlyPromotedStruct) { varDsc->lvMustInit = true; diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index a7ce817092368..7ad716fe79c5a 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7914,12 +7914,13 @@ PhaseStatus Lowering::DoPhase() // void Lowering::MapParameterRegisterLocals() { - comp->m_regParamLocalMappings = new (comp, CMK_ABI) ArrayStack(comp->getAllocator(CMK_ABI)); + comp->m_regParamLocalMappings = + new (comp, CMK_ABI) ArrayStack(comp->getAllocator(CMK_ABI)); // Create initial mappings for promotions. for (unsigned lclNum = 0; lclNum < comp->info.compArgsCount; lclNum++) { - LclVarDsc* lclDsc = comp->lvaGetDesc(lclNum); + LclVarDsc* lclDsc = comp->lvaGetDesc(lclNum); const ABIPassingInformation& abiInfo = comp->lvaGetParameterABIInfo(lclNum); if (abiInfo.HasAnyStackSegment()) @@ -7934,8 +7935,8 @@ void Lowering::MapParameterRegisterLocals() for (int i = 0; i < lclDsc->lvFieldCnt; i++) { - unsigned fieldLclNum = lclDsc->lvFieldLclStart + i; - LclVarDsc* fieldDsc = comp->lvaGetDesc(fieldLclNum); + unsigned fieldLclNum = lclDsc->lvFieldLclStart + i; + LclVarDsc* fieldDsc = comp->lvaGetDesc(fieldLclNum); for (const ABIPassingSegment& segment : abiInfo.Segments()) { diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index f542f245f4b3b..b74dbd824e748 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -2185,10 +2185,12 @@ void LinearScan::buildIntervals() continue; } - const RegisterParameterLocalMapping* mapping = compiler->FindParameterRegisterLocalMappingByRegister(seg.GetRegister()); + const RegisterParameterLocalMapping* mapping = + compiler->FindParameterRegisterLocalMappingByRegister(seg.GetRegister()); LclVarDsc* argDsc = compiler->lvaGetDesc(mapping != nullptr ? mapping->LclNum : lclNum); - if (argDsc->lvTracked && !compiler->compJmpOpUsed && (argDsc->lvRefCnt() == 0) && !compiler->opts.compDbgCode) + if (argDsc->lvTracked && !compiler->compJmpOpUsed && (argDsc->lvRefCnt() == 0) && + !compiler->opts.compDbgCode) { // Not live continue; @@ -2201,7 +2203,7 @@ void LinearScan::buildIntervals() for (unsigned int varIndex = 0; varIndex < compiler->lvaTrackedCount; varIndex++) { - unsigned lclNum = compiler->lvaTrackedIndexToLclNum(varIndex); + unsigned lclNum = compiler->lvaTrackedIndexToLclNum(varIndex); LclVarDsc* lclDsc = compiler->lvaGetDesc(lclNum); if (!isCandidateVar(lclDsc)) @@ -2223,7 +2225,8 @@ void LinearScan::buildIntervals() regNumber paramReg = REG_NA; if (lclDsc->lvIsParamRegTarget) { - const RegisterParameterLocalMapping* mapping = compiler->FindParameterRegisterLocalMappingByLocal(lclNum, 0); + const RegisterParameterLocalMapping* mapping = + compiler->FindParameterRegisterLocalMappingByLocal(lclNum, 0); assert(mapping != nullptr); paramReg = mapping->RegisterSegment->GetRegister(); } From 559baf06bcfa1010fd6c0adca00c6855016f58f0 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 18 Dec 2024 12:16:42 +0100 Subject: [PATCH 6/9] Avoid homing Swift parameters if they are !lvOnFrame --- src/coreclr/jit/codegencommon.cpp | 9 ++++++--- src/coreclr/jit/lsrabuild.cpp | 4 +++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index fa61ae5087e31..e941bdd885912 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -3374,7 +3374,8 @@ void CodeGen::genHomeRegisterParams(regNumber initReg, bool* initRegStillZeroed) } } - if (compiler->info.compPublishStubParam && ((paramRegs & RBM_SECRET_STUB_PARAM) != RBM_NONE)) + if (compiler->info.compPublishStubParam && ((paramRegs & RBM_SECRET_STUB_PARAM) != RBM_NONE) && + compiler->lvaGetDesc(compiler->lvaStubArgumentVar)->lvOnFrame) { GetEmitter()->emitIns_S_R(ins_Store(TYP_I_IMPL), EA_PTRSIZE, REG_SECRET_STUB_PARAM, compiler->lvaStubArgumentVar, 0); @@ -5701,14 +5702,16 @@ void CodeGen::genFnProlog() if (compiler->info.compCallConv == CorInfoCallConvExtension::Swift) { if ((compiler->lvaSwiftSelfArg != BAD_VAR_NUM) && - ((intRegState.rsCalleeRegArgMaskLiveIn & RBM_SWIFT_SELF) != 0)) + ((intRegState.rsCalleeRegArgMaskLiveIn & RBM_SWIFT_SELF) != 0) && + compiler->lvaGetDesc(compiler->lvaSwiftSelfArg)->lvOnFrame) { GetEmitter()->emitIns_S_R(ins_Store(TYP_I_IMPL), EA_PTRSIZE, REG_SWIFT_SELF, compiler->lvaSwiftSelfArg, 0); intRegState.rsCalleeRegArgMaskLiveIn &= ~RBM_SWIFT_SELF; } if ((compiler->lvaSwiftIndirectResultArg != BAD_VAR_NUM) && - ((intRegState.rsCalleeRegArgMaskLiveIn & theFixedRetBuffMask(CorInfoCallConvExtension::Swift)) != 0)) + ((intRegState.rsCalleeRegArgMaskLiveIn & theFixedRetBuffMask(CorInfoCallConvExtension::Swift)) != 0) && + compiler->lvaGetDesc(compiler->lvaSwiftIndirectResultArg)->lvOnFrame) { GetEmitter()->emitIns_S_R(ins_Store(TYP_I_IMPL), EA_PTRSIZE, theFixedRetBuffReg(CorInfoCallConvExtension::Swift), diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index b74dbd824e748..0b5691f72f30c 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -2188,7 +2188,9 @@ void LinearScan::buildIntervals() const RegisterParameterLocalMapping* mapping = compiler->FindParameterRegisterLocalMappingByRegister(seg.GetRegister()); - LclVarDsc* argDsc = compiler->lvaGetDesc(mapping != nullptr ? mapping->LclNum : lclNum); + unsigned mappedLclNum = mapping != nullptr ? mapping->LclNum : lclNum; + JITDUMP("Arg V%02u in reg %s\n", mappedLclNum, getRegName(seg.GetRegister())); + LclVarDsc* argDsc = compiler->lvaGetDesc(mappedLclNum); if (argDsc->lvTracked && !compiler->compJmpOpUsed && (argDsc->lvRefCnt() == 0) && !compiler->opts.compDbgCode) { From 6d1b57ebc126310b2a958d4cfa0d2b680f17d0e9 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 18 Dec 2024 13:11:58 +0100 Subject: [PATCH 7/9] Rename --- src/coreclr/jit/codegencommon.cpp | 2 +- src/coreclr/jit/compiler.cpp | 16 ++++++++-------- src/coreclr/jit/compiler.h | 12 ++++++------ src/coreclr/jit/lower.cpp | 12 ++++++------ src/coreclr/jit/lsrabuild.cpp | 4 ++-- 5 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index e941bdd885912..b6555f4bb1186 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -3407,7 +3407,7 @@ void CodeGen::genHomeRegisterParams(regNumber initReg, bool* initRegStillZeroed) continue; } - const RegisterParameterLocalMapping* mapping = + const ParameterRegisterLocalMapping* mapping = compiler->FindParameterRegisterLocalMappingByRegister(segment.GetRegister()); unsigned fieldLclNum = mapping != nullptr ? mapping->LclNum : lclNum; diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index e1aba64032cec..9ec9c84c785bd 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -4229,16 +4229,16 @@ bool Compiler::compRsvdRegCheck(FrameLayoutState curState) // Returns: // The mapping, or nullptr if no mapping was found for this register. // -const RegisterParameterLocalMapping* Compiler::FindParameterRegisterLocalMappingByRegister(regNumber reg) +const ParameterRegisterLocalMapping* Compiler::FindParameterRegisterLocalMappingByRegister(regNumber reg) { - if (m_regParamLocalMappings == nullptr) + if (m_paramRegLocalMappings == nullptr) { return nullptr; } - for (int i = 0; i < m_regParamLocalMappings->Height(); i++) + for (int i = 0; i < m_paramRegLocalMappings->Height(); i++) { - const RegisterParameterLocalMapping& mapping = m_regParamLocalMappings->BottomRef(i); + const ParameterRegisterLocalMapping& mapping = m_paramRegLocalMappings->BottomRef(i); if (mapping.RegisterSegment->GetRegister() == reg) { return &mapping; @@ -4260,17 +4260,17 @@ const RegisterParameterLocalMapping* Compiler::FindParameterRegisterLocalMapping // Returns: // The mapping, or nullptr if no mapping was found for this local. // -const RegisterParameterLocalMapping* Compiler::FindParameterRegisterLocalMappingByLocal(unsigned lclNum, +const ParameterRegisterLocalMapping* Compiler::FindParameterRegisterLocalMappingByLocal(unsigned lclNum, unsigned offset) { - if (m_regParamLocalMappings == nullptr) + if (m_paramRegLocalMappings == nullptr) { return nullptr; } - for (int i = 0; i < m_regParamLocalMappings->Height(); i++) + for (int i = 0; i < m_paramRegLocalMappings->Height(); i++) { - const RegisterParameterLocalMapping& mapping = m_regParamLocalMappings->BottomRef(i); + const ParameterRegisterLocalMapping& mapping = m_paramRegLocalMappings->BottomRef(i); if ((mapping.LclNum == lclNum) && (mapping.Offset == offset)) { return &mapping; diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 84cc274c15b6e..2622a2c07a5e6 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2582,12 +2582,12 @@ struct CloneTryInfo }; //------------------------------------------------------------------------ -// RegisterParameterLocalMapping: +// ParameterRegisterLocalMapping: // Contains mappings between a parameter register segment and a corresponding // local. Used by the backend to know which locals are expected to contain // which register parameters after the prolog. // -struct RegisterParameterLocalMapping +struct ParameterRegisterLocalMapping { const ABIPassingSegment* RegisterSegment; unsigned LclNum; @@ -2599,7 +2599,7 @@ struct RegisterParameterLocalMapping // xmm0[0..8), xmm1[8..12), but enregistered as one register. unsigned Offset; - RegisterParameterLocalMapping(const ABIPassingSegment* segment, unsigned lclNum, unsigned offset) + ParameterRegisterLocalMapping(const ABIPassingSegment* segment, unsigned lclNum, unsigned offset) : RegisterSegment(segment) , LclNum(lclNum) , Offset(offset) @@ -8320,10 +8320,10 @@ class Compiler LinearScanInterface* m_pLinearScan = nullptr; // Linear Scan allocator public: - ArrayStack* m_regParamLocalMappings = nullptr; + ArrayStack* m_paramRegLocalMappings = nullptr; - const RegisterParameterLocalMapping* FindParameterRegisterLocalMappingByRegister(regNumber reg); - const RegisterParameterLocalMapping* FindParameterRegisterLocalMappingByLocal(unsigned lclNum, unsigned offset); + const ParameterRegisterLocalMapping* FindParameterRegisterLocalMappingByRegister(regNumber reg); + const ParameterRegisterLocalMapping* FindParameterRegisterLocalMappingByLocal(unsigned lclNum, unsigned offset); /* XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 7ad716fe79c5a..8b7b3dd890cd8 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7914,8 +7914,8 @@ PhaseStatus Lowering::DoPhase() // void Lowering::MapParameterRegisterLocals() { - comp->m_regParamLocalMappings = - new (comp, CMK_ABI) ArrayStack(comp->getAllocator(CMK_ABI)); + comp->m_paramRegLocalMappings = + new (comp, CMK_ABI) ArrayStack(comp->getAllocator(CMK_ABI)); // Create initial mappings for promotions. for (unsigned lclNum = 0; lclNum < comp->info.compArgsCount; lclNum++) @@ -7950,7 +7950,7 @@ void Lowering::MapParameterRegisterLocals() continue; } - comp->m_regParamLocalMappings->Emplace(&segment, fieldLclNum, segment.Offset - fieldDsc->lvFldOffset); + comp->m_paramRegLocalMappings->Emplace(&segment, fieldLclNum, segment.Offset - fieldDsc->lvFldOffset); } LclVarDsc* fieldLclDsc = comp->lvaGetDesc(fieldLclNum); @@ -7962,10 +7962,10 @@ void Lowering::MapParameterRegisterLocals() #ifdef DEBUG if (comp->verbose) { - printf("%d parameter register to local mappings\n", comp->m_regParamLocalMappings->Height()); - for (int i = 0; i < comp->m_regParamLocalMappings->Height(); i++) + printf("%d parameter register to local mappings\n", comp->m_paramRegLocalMappings->Height()); + for (int i = 0; i < comp->m_paramRegLocalMappings->Height(); i++) { - const RegisterParameterLocalMapping& mapping = comp->m_regParamLocalMappings->BottomRef(i); + const ParameterRegisterLocalMapping& mapping = comp->m_paramRegLocalMappings->BottomRef(i); printf(" "); mapping.RegisterSegment->Dump(); printf(" -> V%02u\n", mapping.LclNum); diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 0b5691f72f30c..4cf182f374ebb 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -2185,7 +2185,7 @@ void LinearScan::buildIntervals() continue; } - const RegisterParameterLocalMapping* mapping = + const ParameterRegisterLocalMapping* mapping = compiler->FindParameterRegisterLocalMappingByRegister(seg.GetRegister()); unsigned mappedLclNum = mapping != nullptr ? mapping->LclNum : lclNum; @@ -2227,7 +2227,7 @@ void LinearScan::buildIntervals() regNumber paramReg = REG_NA; if (lclDsc->lvIsParamRegTarget) { - const RegisterParameterLocalMapping* mapping = + const ParameterRegisterLocalMapping* mapping = compiler->FindParameterRegisterLocalMappingByLocal(lclNum, 0); assert(mapping != nullptr); paramReg = mapping->RegisterSegment->GetRegister(); From 31b10eeb2570fa7e716a865a792efdeb3d3e7b38 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Fri, 20 Dec 2024 12:10:54 +0100 Subject: [PATCH 8/9] Address feedback, port back some changes from upcoming PR --- src/coreclr/jit/codegen.h | 7 +++--- src/coreclr/jit/codegencommon.cpp | 39 +++++++++++++++++-------------- src/coreclr/jit/lower.cpp | 22 +++++++++++++++-- src/coreclr/jit/lsrabuild.cpp | 8 ++++++- 4 files changed, 53 insertions(+), 23 deletions(-) diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index 8d6d2ef5684cc..75694d30a3819 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -260,9 +260,10 @@ class CodeGen final : public CodeGenInterface regMaskTP genGetParameterHomingTempRegisterCandidates(); var_types genParamStackType(LclVarDsc* dsc, const ABIPassingSegment& seg); - void genSpillOrAddRegisterParam(unsigned lclNum, const ABIPassingSegment& seg, class RegGraph* graph); - void genSpillOrAddNonStandardRegisterParam(unsigned lclNum, regNumber sourceReg, class RegGraph* graph); - void genEnregisterIncomingStackArgs(); + void genSpillOrAddRegisterParam( + unsigned lclNum, unsigned offset, unsigned paramLclNum, const ABIPassingSegment& seg, class RegGraph* graph); + void genSpillOrAddNonStandardRegisterParam(unsigned lclNum, regNumber sourceReg, class RegGraph* graph); + void genEnregisterIncomingStackArgs(); #if defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64) void genEnregisterOSRArgsAndLocals(regNumber initReg, bool* pInitRegZeroed); #else diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index b6555f4bb1186..5024b61596ef9 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -3239,26 +3239,25 @@ var_types CodeGen::genParamStackType(LclVarDsc* dsc, const ABIPassingSegment& se // to stack immediately, or by adding it to the register graph. // // Parameters: -// lclNum - Parameter local (or field of it) -// segment - Register segment to either spill or put in the register graph -// graph - The register graph to add to -// -void CodeGen::genSpillOrAddRegisterParam(unsigned lclNum, const ABIPassingSegment& segment, RegGraph* graph) +// lclNum - Target local +// offset - Offset into the target local +// paramLclNum - Local that is the actual parameter that has the incoming register +// segment - Register segment to either spill or put in the register graph +// graph - The register graph to add to +// +void CodeGen::genSpillOrAddRegisterParam( + unsigned lclNum, unsigned offset, unsigned paramLclNum, const ABIPassingSegment& segment, RegGraph* graph) { - regMaskTP paramRegs = intRegState.rsCalleeRegArgMaskLiveIn | floatRegState.rsCalleeRegArgMaskLiveIn; - LclVarDsc* varDsc = compiler->lvaGetDesc(lclNum); - - unsigned baseOffset = varDsc->lvIsStructField ? varDsc->lvFldOffset : 0; - unsigned size = varDsc->lvExactSize(); + regMaskTP paramRegs = intRegState.rsCalleeRegArgMaskLiveIn | floatRegState.rsCalleeRegArgMaskLiveIn; if (!segment.IsPassedInRegister() || ((paramRegs & genRegMask(segment.GetRegister())) == 0)) { return; } + LclVarDsc* varDsc = compiler->lvaGetDesc(lclNum); if (varDsc->lvOnFrame && (!varDsc->lvIsInReg() || varDsc->lvLiveInOutOfHndlr)) { - unsigned paramLclNum = varDsc->lvIsStructField ? varDsc->lvParentLcl : lclNum; LclVarDsc* paramVarDsc = compiler->lvaGetDesc(paramLclNum); var_types storeType = genParamStackType(paramVarDsc, segment); @@ -3269,7 +3268,7 @@ void CodeGen::genSpillOrAddRegisterParam(unsigned lclNum, const ABIPassingSegmen } GetEmitter()->emitIns_S_R(ins_Store(storeType), emitActualTypeSize(storeType), segment.GetRegister(), lclNum, - segment.Offset - baseOffset); + offset); } if (!varDsc->lvIsInReg()) @@ -3289,12 +3288,12 @@ void CodeGen::genSpillOrAddRegisterParam(unsigned lclNum, const ABIPassingSegmen RegNode* sourceReg = graph->GetOrAdd(segment.GetRegister()); RegNode* destReg = graph->GetOrAdd(varDsc->GetRegNum()); - if ((sourceReg != destReg) || (baseOffset != segment.Offset)) + if ((sourceReg != destReg) || (offset != 0)) { #ifdef TARGET_ARM if (edgeType == TYP_DOUBLE) { - assert(segment.Offset == baseOffset); + assert(offset == 0); graph->AddEdge(sourceReg, destReg, TYP_FLOAT, 0); sourceReg = graph->GetOrAdd(REG_NEXT(sourceReg->reg)); @@ -3303,7 +3302,7 @@ void CodeGen::genSpillOrAddRegisterParam(unsigned lclNum, const ABIPassingSegmen return; } #endif - graph->AddEdge(sourceReg, destReg, edgeType, segment.Offset - baseOffset); + graph->AddEdge(sourceReg, destReg, edgeType, offset); } } @@ -3410,8 +3409,14 @@ void CodeGen::genHomeRegisterParams(regNumber initReg, bool* initRegStillZeroed) const ParameterRegisterLocalMapping* mapping = compiler->FindParameterRegisterLocalMappingByRegister(segment.GetRegister()); - unsigned fieldLclNum = mapping != nullptr ? mapping->LclNum : lclNum; - genSpillOrAddRegisterParam(fieldLclNum, segment, &graph); + if (mapping != nullptr) + { + genSpillOrAddRegisterParam(mapping->LclNum, mapping->Offset, lclNum, segment, &graph); + } + else + { + genSpillOrAddRegisterParam(lclNum, segment.Offset, lclNum, segment, &graph); + } } } diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 8b7b3dd890cd8..c58e7d1cf3635 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7923,16 +7923,25 @@ void Lowering::MapParameterRegisterLocals() LclVarDsc* lclDsc = comp->lvaGetDesc(lclNum); const ABIPassingInformation& abiInfo = comp->lvaGetParameterABIInfo(lclNum); - if (abiInfo.HasAnyStackSegment()) + if (comp->lvaGetPromotionType(lclDsc) != Compiler::PROMOTION_TYPE_INDEPENDENT) { + // If not promoted, then we do not need to create any mappings. + // If dependently promoted then the fields are never enregistered + // by LSRA, so no reason to try to create any mappings. continue; } - if (comp->lvaGetPromotionType(lclDsc) != Compiler::PROMOTION_TYPE_INDEPENDENT) + if (!abiInfo.HasAnyRegisterSegment()) { + // If the parameter is not passed in any registers, then there are + // no mappings to create. continue; } + // Currently we do not support promotion of split parameters, so we + // should not see any split parameters here. + assert(!abiInfo.IsSplitAcrossRegistersAndStack()); + for (int i = 0; i < lclDsc->lvFieldCnt; i++) { unsigned fieldLclNum = lclDsc->lvFieldLclStart + i; @@ -7942,14 +7951,23 @@ void Lowering::MapParameterRegisterLocals() { if (segment.Offset + segment.Size <= fieldDsc->lvFldOffset) { + // This register does not map to this field (ends before the field starts) continue; } if (fieldDsc->lvFldOffset + fieldDsc->lvExactSize() <= segment.Offset) { + // This register does not map to this field (starts after the field ends) continue; } + // Register is inside the field. We always expect it to be + // fully contained; we do not promote when it isn't. However, + // we may promote in cases where multiple registers map to the + // same field. + assert((segment.Offset >= fieldDsc->lvFldOffset) && + (segment.Offset + segment.Size <= fieldDsc->lvFldOffset + genTypeSize(fieldDsc))); + comp->m_paramRegLocalMappings->Emplace(&segment, fieldLclNum, segment.Offset - fieldDsc->lvFldOffset); } diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 4cf182f374ebb..72f88864d755a 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -2175,6 +2175,8 @@ void LinearScan::buildIntervals() regsInUseThisLocation = RBM_NONE; regsInUseNextLocation = RBM_NONE; + // Compute live incoming parameter registers. The liveness is based on the + // locals we are expecting to store the registers into in the prolog. for (unsigned lclNum = 0; lclNum < compiler->info.compArgsCount; lclNum++) { const ABIPassingInformation& abiInfo = compiler->lvaGetParameterABIInfo(lclNum); @@ -2203,6 +2205,8 @@ void LinearScan::buildIntervals() } } + // Now build initial definitions for all parameters, preferring their ABI + // register if passed in one. for (unsigned int varIndex = 0; varIndex < compiler->lvaTrackedCount; varIndex++) { unsigned lclNum = compiler->lvaTrackedIndexToLclNum(varIndex); @@ -2227,6 +2231,7 @@ void LinearScan::buildIntervals() regNumber paramReg = REG_NA; if (lclDsc->lvIsParamRegTarget) { + // Prefer the first ABI register. const ParameterRegisterLocalMapping* mapping = compiler->FindParameterRegisterLocalMappingByLocal(lclNum, 0); assert(mapping != nullptr); @@ -2236,7 +2241,7 @@ void LinearScan::buildIntervals() { if (lclDsc->lvIsStructField) { - // All promoted fields should be assigned via the + // All fields passed in registers should be assigned via the // lvIsParamRegTarget mechanism, so this must be a stack // argument. assert(!compiler->lvaGetParameterABIInfo(lclDsc->lvParentLcl).HasAnyRegisterSegment()); @@ -2260,6 +2265,7 @@ void LinearScan::buildIntervals() } else { + // Not a parameter or target of a parameter register continue; } From a4fa31089a935917ac7d707d5b93fe7780a590ee Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Fri, 20 Dec 2024 13:09:19 +0100 Subject: [PATCH 9/9] Remove false assert --- src/coreclr/jit/lower.cpp | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index c58e7d1cf3635..b11495b871392 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7961,13 +7961,6 @@ void Lowering::MapParameterRegisterLocals() continue; } - // Register is inside the field. We always expect it to be - // fully contained; we do not promote when it isn't. However, - // we may promote in cases where multiple registers map to the - // same field. - assert((segment.Offset >= fieldDsc->lvFldOffset) && - (segment.Offset + segment.Size <= fieldDsc->lvFldOffset + genTypeSize(fieldDsc))); - comp->m_paramRegLocalMappings->Emplace(&segment, fieldLclNum, segment.Offset - fieldDsc->lvFldOffset); }