diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index f7d9192cb588ec..1861143ca2a555 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -1133,6 +1133,8 @@ class CodeGen final : public CodeGenInterface unsigned varNum, var_types type, GenTreeLclVar* lclNode, regNumber regNum, bool reSpill, bool isLastUse); void genUnspillRegIfNeeded(GenTree* tree); void genUnspillRegIfNeeded(GenTree* tree, unsigned multiRegIndex); + bool isRematerializableConstant(GenTree* tree); + bool isRematerializedConstantSpill(GenTree* tree); regNumber genConsumeReg(GenTree* tree); regNumber genConsumeReg(GenTree* tree, unsigned multiRegIndex); void genCopyRegIfNeeded(GenTree* tree, regNumber needReg); diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index 5188908e03f48e..5491c96a8eabb8 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -192,7 +192,14 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) #if defined(FEATURE_MASKED_HW_INTRINSICS) case GT_CNS_MSK: #endif // FEATURE_MASKED_HW_INTRINSICS - genSetRegToConst(targetReg, targetType, treeNode); + // A rematerializable constant that is spilled right after its definition has no + // in-register use; its value is rematerialized at each reload point (see + // genUnspillRegIfNeeded) and its store is skipped (see genProduceReg). Skip emitting + // the definition too, since it would only materialize a value that is never read. + if (!isRematerializedConstantSpill(treeNode)) + { + genSetRegToConst(targetReg, targetType, treeNode); + } genProduceReg(treeNode); break; diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index 49f67585225024..2eb5e832c077a1 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -1394,21 +1394,180 @@ void CodeGen::genUnspillRegIfNeeded(GenTree* tree) } else { - // Here we may have a GT_RELOAD. - // The spill temp allocated for it is associated with the original tree that defined the - // register that it was spilled from. - // So we use 'unspillTree' to recover that spill temp. - TempDsc* t = regSet.rsUnspillInPlace(unspillTree, unspillTree->GetRegNum()); - emitAttr emitType = emitActualTypeSize(unspillTree->TypeGet()); - // Reload into the register specified by 'tree' which may be a GT_RELOAD. regNumber dstReg = tree->GetRegNum(); - GetEmitter()->emitIns_R_S(ins_Load(unspillTree->gtType), emitType, dstReg, t->tdTempNum(), 0); - regSet.tmpRlsTemp(t); - unspillTree->gtFlags &= ~GTF_SPILLED; - gcInfo.gcMarkRegPtrVal(dstReg, unspillTree->TypeGet()); +#if defined(TARGET_XARCH) || defined(TARGET_ARM64) + // A floating-point, SIMD, or mask constant is not spilled to the stack (see + // genProduceReg); rematerialize it directly into the reload target register rather + // than loading it from a spill temp that was never created. + if (isRematerializableConstant(unspillTree) && ((unspillTree->gtFlags & GTF_NOREG_AT_USE) == 0)) + { +#if defined(TARGET_XARCH) + // The GT_CNS_VEC and GT_CNS_MSK GenTree* overloads of genSetRegToConst materialize + // into the node's own register, so the explicit-register overloads are used to honor + // 'dstReg' (which may be a GT_RELOAD target register that differs from the original + // definition's register). + switch (unspillTree->OperGet()) + { + case GT_CNS_DBL: + genSetRegToConst(dstReg, unspillTree->TypeGet(), unspillTree); + break; + case GT_CNS_VEC: + genSetRegToConst(dstReg, unspillTree->TypeGet(), &unspillTree->AsVecCon()->gtSimdVal); + break; + case GT_CNS_MSK: + genSetRegToConst(dstReg, unspillTree->TypeGet(), &unspillTree->AsMskCon()->gtSimdMaskVal); + break; + default: + unreached(); + } +#else // TARGET_ARM64 + // The arm64 genSetRegToConst honors the explicit target register for every operator. + genSetRegToConst(dstReg, unspillTree->TypeGet(), unspillTree); +#endif // TARGET_XARCH + + unspillTree->gtFlags &= ~GTF_SPILLED; + } + else +#endif // TARGET_XARCH || TARGET_ARM64 + { + // Here we may have a GT_RELOAD. + // The spill temp allocated for it is associated with the original tree that defined the + // register that it was spilled from. + // So we use 'unspillTree' to recover that spill temp. + TempDsc* t = regSet.rsUnspillInPlace(unspillTree, unspillTree->GetRegNum()); + emitAttr emitType = emitActualTypeSize(unspillTree->TypeGet()); + // Reload into the register specified by 'tree' which may be a GT_RELOAD. + GetEmitter()->emitIns_R_S(ins_Load(unspillTree->gtType), emitType, dstReg, t->tdTempNum(), 0); + regSet.tmpRlsTemp(t); + + unspillTree->gtFlags &= ~GTF_SPILLED; + gcInfo.gcMarkRegPtrVal(dstReg, unspillTree->TypeGet()); + } + } + } +} + +//------------------------------------------------------------------------ +// isRematerializableConstant: Determine whether a spilled constant tree temp can be +// rematerialized at its reload point instead of being spilled to and reloaded +// from the stack. +// +// Arguments: +// tree - the node that defines the value +// +// Return Value: +// True if 'tree' is a floating-point, SIMD, or mask constant that this target can +// rematerialize without requiring a scratch register or GC bookkeeping. +// +// Notes: +// These constants have no GC liveness. On xarch every such constant can be +// rematerialized without a scratch register (zero via `xorps`, all-bits-set via +// `pcmpeqd`/`vpternlogd`, or a RIP-relative load from the constant's memory home), +// so all of them are eligible. +// +// On arm64 only the subset that genSetRegToConst encodes directly into an instruction +// is eligible; the remaining values are loaded from the constant pool via `adrp`/`ldr` +// which needs a scratch address register that is not reserved at the reload point. The +// conditions below must therefore stay in sync with the direct-encoding fast paths in +// genSetRegToConst: 0.0/fmov-immediate for CNS_DBL, zero/all-bits-set/movi-broadcast for +// CNS_VEC, and every CNS_MSK (encoded via `pfalse`/`ptrue`, which never needs a scratch). +// Other targets can be enabled later for their own scratch-free subset. +// +bool CodeGen::isRematerializableConstant(GenTree* tree) +{ +#if defined(TARGET_XARCH) + return tree->OperIs(GT_CNS_DBL, GT_CNS_VEC, GT_CNS_MSK); +#elif defined(TARGET_ARM64) + switch (tree->OperGet()) + { + case GT_CNS_DBL: + { + double constValue = tree->AsDblCon()->DconValue(); + return (*(int64_t*)&constValue == 0) || emitter::emitIns_valid_imm_for_fmov(constValue); + } + +#if defined(FEATURE_SIMD) + case GT_CNS_VEC: + { + // genSetRegToConst only encodes SIMD8/12/16 constants without a scratch register; any + // wider type falls through to its 'unreached' path, so reject those up front (mirroring + // the type switch in genSetRegToConst). + if (!tree->TypeIs(TYP_SIMD8, TYP_SIMD12, TYP_SIMD16)) + { + return false; + } + + GenTreeVecCon* vecCon = tree->AsVecCon(); + + if (vecCon->IsAllBitsSet() || vecCon->IsZero()) + { + return true; + } + + const bool is8 = tree->TypeIs(TYP_SIMD8); + simd16_t val = vecCon->gtSimd16Val; + + return (ElementsAreSame(val.i32, is8 ? 2 : 4) && + emitter::emitIns_valid_imm_for_movi(val.i32[0], EA_4BYTE)) || + (ElementsAreSame(val.i16, is8 ? 4 : 8) && + emitter::emitIns_valid_imm_for_movi(val.i16[0], EA_2BYTE)) || + (ElementsAreSame(val.i8, is8 ? 8 : 16) && emitter::emitIns_valid_imm_for_movi(val.i8[0], EA_1BYTE)); } +#endif // FEATURE_SIMD + +#if defined(FEATURE_MASKED_HW_INTRINSICS) + case GT_CNS_MSK: + return true; +#endif // FEATURE_MASKED_HW_INTRINSICS + + default: + return false; + } +#else + return false; +#endif // TARGET_XARCH +} + +//------------------------------------------------------------------------ +// isRematerializedConstantSpill: Determine whether a constant that LSRA has marked to +// spill will instead be handled entirely by rematerialization, so that both its stack +// store and its register definition can be elided. +// +// Arguments: +// tree - the node that defines the value +// +// Return Value: +// True if 'tree' is a rematerializable constant whose definition is spilled immediately +// (GTF_SPILL) and whose value never needs to live in a stack spill temp, so both the store +// and the register definition can be elided. +// +// Notes: +// GTF_SPILL on a definition carries spill-after semantics: LSRA stores the value right +// after it is produced and frees the register, so there is no in-register use of the +// definition. When such a value is rematerializable, the stack store is skipped (see +// genProduceReg) and the definition itself only materializes a value into a register that +// is never read, so callers use this predicate to also elide the definition. +// +// A reload that needs the value in a register rematerializes it (see genUnspillRegIfNeeded). +// A use that instead consumes the value directly from memory is marked GTF_NOREG_AT_USE; on +// xarch that memory operand can be the constant's data-section home (see genOperandDesc and +// emitInsBinary), so the definition and its spill store are still unnecessary. Targets that +// cannot encode the constant as a memory operand must materialize it into a register at the +// use, so there the definition and its store are retained for the GTF_NOREG_AT_USE case. +// +bool CodeGen::isRematerializedConstantSpill(GenTree* tree) +{ + if (((tree->gtFlags & GTF_SPILL) == 0) || !isRematerializableConstant(tree)) + { + return false; } + +#if defined(TARGET_XARCH) + return true; +#else + return (tree->gtFlags & GTF_NOREG_AT_USE) == 0; +#endif } //------------------------------------------------------------------------ @@ -2232,6 +2391,21 @@ void CodeGen::genProduceReg(GenTree* tree) } else { + // Floating-point, SIMD, and mask constants have no GC liveness and (on the targets + // for which isRematerializableConstant is enabled) can be rematerialized cheaply + // without a scratch register. Rather than spilling such a value to the stack, skip + // creating the spill temp and emitting the store; a reload rematerializes it into a + // register (see genUnspillRegIfNeeded) and, on xarch, a use that consumes it from + // memory reads the constant's data-section home directly (see genOperandDesc). The + // definition that produced this value is likewise elided at the definition site (see + // genCodeForTreeNode), so nothing reads the register here. + if (isRematerializedConstantSpill(tree)) + { + tree->gtFlags |= GTF_SPILLED; + tree->gtFlags &= ~GTF_SPILL; + return; + } + regSet.rsSpillTree(tree->GetRegNum(), tree); gcInfo.gcMarkRegSetNpt(genRegMask(tree->GetRegNum())); } diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index 5308e680efa374..dfad1e08751619 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -1846,7 +1846,15 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) #if defined(FEATURE_MASKED_HW_INTRINSICS) case GT_CNS_MSK: #endif // FEATURE_MASKED_HW_INTRINSICS - genSetRegToConst(targetReg, targetType, treeNode); + // A rematerializable constant that is spilled right after its definition has no + // in-register use; a reload rematerializes it (see genUnspillRegIfNeeded) or a use + // consumes it from its data-section home (see genOperandDesc), and its store is skipped + // (see genProduceReg). Skip emitting the definition too, since it would only materialize + // a value that is never read. + if (!isRematerializedConstantSpill(treeNode)) + { + genSetRegToConst(targetReg, targetType, treeNode); + } genProduceReg(treeNode); break; diff --git a/src/coreclr/jit/emit.cpp b/src/coreclr/jit/emit.cpp index af557d5d49de3b..c48990c263549c 100644 --- a/src/coreclr/jit/emit.cpp +++ b/src/coreclr/jit/emit.cpp @@ -6971,7 +6971,6 @@ unsigned emitter::emitEndCodeGen(Compiler* comp, AllocMemChunk* dataChunk = emitDataChunks; unsigned* dataChunkOffset = emitDataChunkOffsets; - unsigned cumulativeOffset = 0; for (dataSection* sec = emitConsDsc.dsdList; sec != nullptr; sec = sec->dsNext, dataChunk++, dataChunkOffset++) { comp->Metrics.ReadOnlyDataBytes += sec->dsSize; @@ -6985,8 +6984,10 @@ unsigned emitter::emitEndCodeGen(Compiler* comp, dataChunk->flags = CORJIT_ALLOCMEM_READONLY_DATA | CORJIT_ALLOCMEM_HAS_POINTERS_TO_CODE; } - *dataChunkOffset = cumulativeOffset; - cumulativeOffset += sec->dsSize; + // The logical offset assigned to each section (see emitDataGenBeg) is what instructions + // reference and what emitDataOffsetToPtr maps back to a chunk, so use it here rather than + // recomputing a packed offset that would ignore inter-section alignment padding. + *dataChunkOffset = sec->dsOffset; } comp->Metrics.AllocatedHotCodeBytes = emitTotalHotCodeSize; @@ -7873,9 +7874,15 @@ UNATIVE_OFFSET emitter::emitDataGenBeg(unsigned size, unsigned alignment, var_ty // assert((size != 0) && ((size % dataSection::MIN_DATA_ALIGN) == 0)); - unsigned secOffs = emitConsDsc.dsdOffs; + // Place the constant at an offset that satisfies its required alignment. This ensures + // aligned loads of the constant address a properly aligned location, and it lets the + // duplicate-constant matching in emitDataGenFind reuse an existing entry (that search + // only considers candidates sitting at a suitably aligned offset). Compilers targeting + // SMALL_CODE request a smaller alignment in the const-emission helpers and so still get + // tight packing. + unsigned secOffs = AlignUp(emitConsDsc.dsdOffs, alignment); /* Advance the current offset */ - emitConsDsc.dsdOffs += size; + emitConsDsc.dsdOffs = secOffs + size; /* Allocate a data section descriptor and add it to the list */ @@ -7887,6 +7894,8 @@ UNATIVE_OFFSET emitter::emitDataGenBeg(unsigned size, unsigned alignment, var_ty secDesc->dsAlignment = alignment; + secDesc->dsOffset = secOffs; + secDesc->dsDataType = dataType; secDesc->dsNext = nullptr; @@ -7922,13 +7931,13 @@ UNATIVE_OFFSET emitter::emitBBTableDataGenBeg(unsigned numEntries, bool relative UNATIVE_OFFSET emittedSize = numEntries * elemSize; - /* Get hold of the current offset */ + /* Get hold of the current offset, aligned for the element size */ - secOffs = emitConsDsc.dsdOffs; + secOffs = AlignUp(emitConsDsc.dsdOffs, elemSize); /* Advance the current offset */ - emitConsDsc.dsdOffs += emittedSize; + emitConsDsc.dsdOffs = secOffs + emittedSize; /* Allocate a data section descriptor and add it to the list */ @@ -7940,6 +7949,8 @@ UNATIVE_OFFSET emitter::emitBBTableDataGenBeg(unsigned numEntries, bool relative secDesc->dsAlignment = elemSize; + secDesc->dsOffset = secOffs; + secDesc->dsDataType = TYP_UNKNOWN; secDesc->dsNext = nullptr; @@ -7969,9 +7980,9 @@ UNATIVE_OFFSET emitter::emitBBTableDataGenBeg(unsigned numEntries, bool relative // void emitter::emitAsyncResumeTable(unsigned numEntries, UNATIVE_OFFSET* dataSecOffs, emitter::dataSection** dataSec) { - UNATIVE_OFFSET secOffs = emitConsDsc.dsdOffs; unsigned emittedSize = sizeof(CORINFO_AsyncResumeInfo) * numEntries; - emitConsDsc.dsdOffs += emittedSize; + UNATIVE_OFFSET secOffs = AlignUp(emitConsDsc.dsdOffs, TARGET_POINTER_SIZE); + emitConsDsc.dsdOffs = secOffs + emittedSize; dataSection* secDesc = (dataSection*)emitGetMem(sizeof(dataSection)); secDesc->dsType = dataSection::asyncResumeInfo; @@ -7982,6 +7993,7 @@ void emitter::emitAsyncResumeTable(unsigned numEntries, UNATIVE_OFFSET* dataSecO secDesc->dsSize = emittedSize; secDesc->dsAlignment = TARGET_POINTER_SIZE; + secDesc->dsOffset = secOffs; secDesc->dsDataType = TYP_UNKNOWN; secDesc->dsNext = nullptr; @@ -8068,7 +8080,6 @@ UNATIVE_OFFSET emitter::emitDataGenFind(const void* cnsAddr, unsigned cnsSize, u { UNATIVE_OFFSET cnum = INVALID_UNATIVE_OFFSET; unsigned cmpCount = 0; - unsigned curOffs = 0; dataSection* secDesc = emitConsDsc.dsdList; while (secDesc != nullptr) { @@ -8078,11 +8089,17 @@ UNATIVE_OFFSET emitter::emitDataGenFind(const void* cnsAddr, unsigned cnsSize, u // We match the bit pattern, so the dataType can be different // Only match constants when the dsType is 'data' // - if ((secDesc->dsType == dataSection::data) && (secDesc->dsSize >= cnsSize) && ((curOffs % alignment) == 0)) + // The existing entry must also sit at an offset that satisfies the requested alignment; + // emitDataGenBeg aligns every entry's offset to its own alignment, so this normally holds + // for equally-aligned constants but can still fail when reusing a more-strictly-aligned + // request against a less-aligned entry. + // + if ((secDesc->dsType == dataSection::data) && (secDesc->dsSize >= cnsSize) && + ((secDesc->dsOffset % alignment) == 0)) { if (memcmp(cnsAddr, secDesc->Data(), cnsSize) == 0) { - cnum = curOffs; + cnum = secDesc->dsOffset; // We also might want to update the dsDataType // @@ -8099,7 +8116,6 @@ UNATIVE_OFFSET emitter::emitDataGenFind(const void* cnsAddr, unsigned cnsSize, u } } - curOffs += secDesc->dsSize; secDesc = secDesc->dsNext; if (++cmpCount > 64) @@ -8443,8 +8459,7 @@ void emitter::emitOutputDataSec(dataSecDsc* sec, AllocMemChunk* chunks) /* Walk and emit the contents of all the data blocks */ - size_t curOffs = 0; - AllocMemChunk* chunk = chunks; + AllocMemChunk* chunk = chunks; for (dataSection* dsc = sec->dsdList; dsc; dsc = dsc->dsNext, chunk++) { @@ -8546,7 +8561,7 @@ void emitter::emitOutputDataSec(dataSecDsc* sec, AllocMemChunk* chunks) #ifdef DEBUG if (EMITVERBOSE) { - printf(" section %3u, size %2u, RWD%2u:\t", secNum++, dscSize, curOffs); + printf(" section %3u, size %zu, RWD%zu:\t", secNum++, dscSize, (size_t)dsc->dsOffset); for (size_t i = 0; i < dscSize; i++) { @@ -8572,8 +8587,6 @@ void emitter::emitOutputDataSec(dataSecDsc* sec, AllocMemChunk* chunks) } #endif // DEBUG } - - curOffs += dscSize; } } @@ -8593,8 +8606,7 @@ void emitter::emitDispDataSec(dataSecDsc* section, AllocMemChunk* dataChunks) { printf("\n"); - unsigned offset = 0; - AllocMemChunk* chunk = dataChunks; + AllocMemChunk* chunk = dataChunks; for (dataSection* data = section->dsdList; data != nullptr; data = data->dsNext, chunk++) { @@ -8607,9 +8619,8 @@ void emitter::emitDispDataSec(dataSecDsc* section, AllocMemChunk* dataChunks) const char* labelFormat = "%-7s"; char label[64]; - sprintf_s(label, ArrLen(label), "RWD%02u", offset); + sprintf_s(label, ArrLen(label), "RWD%02zu", (size_t)data->dsOffset); printf(labelFormat, label); - offset += data->dsSize; if ((data->dsType == dataSection::blockRelative32) || (data->dsType == dataSection::blockAbsoluteAddr)) { diff --git a/src/coreclr/jit/emit.h b/src/coreclr/jit/emit.h index 780f06bd1b138c..76d557fdc52773 100644 --- a/src/coreclr/jit/emit.h +++ b/src/coreclr/jit/emit.h @@ -3627,6 +3627,7 @@ class emitter dataSection* dsNext; UNATIVE_OFFSET dsAlignment; + UNATIVE_OFFSET dsOffset; UNATIVE_OFFSET dsSize; sectionType dsType; var_types dsDataType; diff --git a/src/coreclr/jit/emitarm.cpp b/src/coreclr/jit/emitarm.cpp index 0fa1e878c73745..9dee7e935c8ac2 100644 --- a/src/coreclr/jit/emitarm.cpp +++ b/src/coreclr/jit/emitarm.cpp @@ -7259,24 +7259,19 @@ void emitter::emitDispInsHelp( emitDispReg(id->idReg1(), attr, true); imm = emitGetInsSC(id); { - dataSection* jdsc = nullptr; - NATIVE_OFFSET offs = 0; + dataSection* jdsc = nullptr; /* Find the appropriate entry in the data section list */ for (jdsc = emitConsDsc.dsdList; jdsc; jdsc = jdsc->dsNext) { - UNATIVE_OFFSET size = jdsc->dsSize; - /* Is this a label table? */ if (jdsc->dsType == dataSection::blockAbsoluteAddr) { - if (offs == imm) + if (jdsc->dsOffset == (UNATIVE_OFFSET)imm) break; } - - offs += size; } if (id->idIsDspReloc()) diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp index 2d8b9328edcbb8..d6d5f759200761 100644 --- a/src/coreclr/jit/emitxarch.cpp +++ b/src/coreclr/jit/emitxarch.cpp @@ -6515,7 +6515,12 @@ regNumber emitter::emitInsBinary(instruction ins, emitAttr attr, GenTree* dst, G assert(!dst->isUsedFromMemory()); otherOp = dst; - if ((src->IsCnsIntOrI() || src->IsCnsFltOrDbl()) && !src->isUsedFromSpillTemp()) + // A floating-point constant is always emitted from its data-section home. When it is + // contained it has no spill temp, and a rematerializable constant that LSRA spilled with + // its use consuming the value from memory (GTF_NOREG_AT_USE) likewise has no spill temp + // (see genProduceReg), so route both to the constant (data section) path below rather + // than trying to read a spill temp that was never created. + if ((src->IsCnsIntOrI() && !src->isUsedFromSpillTemp()) || src->IsCnsFltOrDbl()) { assert(!src->isUsedFromMemory() || src->IsCnsFltOrDbl()); cnsOp = src; diff --git a/src/coreclr/jit/hwintrinsiccodegenxarch.cpp b/src/coreclr/jit/hwintrinsiccodegenxarch.cpp index 2863faecba5df4..ae58dca16cf6b5 100644 --- a/src/coreclr/jit/hwintrinsiccodegenxarch.cpp +++ b/src/coreclr/jit/hwintrinsiccodegenxarch.cpp @@ -2447,9 +2447,6 @@ void CodeGen::genBaseIntrinsic(GenTreeHWIntrinsic* node, insOpts instOptions) { divTypeSize = EA_32BYTE; } - simd_t negOneIntVec = simd_t::AllBitsSet(); - CORINFO_FIELD_HANDLE negOneFld = emit->emitSimdConst(&negOneIntVec, typeSize); - // div-by-zero check emit->emitIns_SIMD_R_R_R(INS_xorpd, typeSize, tmpReg2, tmpReg2, tmpReg2, instOptions); emit->emitIns_SIMD_R_R_R(INS_pcmpeqd, typeSize, tmpReg2, tmpReg2, op2Reg, instOptions); @@ -2467,6 +2464,9 @@ void CodeGen::genBaseIntrinsic(GenTreeHWIntrinsic* node, insOpts instOptions) } CORINFO_FIELD_HANDLE minValueFld = emit->emitSimdConst(&minValueInt, typeSize); + simd_t negOneIntVec = simd_t::AllBitsSet(); + CORINFO_FIELD_HANDLE negOneFld = emit->emitSimdConst(&negOneIntVec, typeSize); + emit->emitIns_SIMD_R_R_C(INS_pcmpeqd, typeSize, tmpReg2, op1Reg, minValueFld, 0, instOptions); emit->emitIns_SIMD_R_R_C(INS_pcmpeqd, typeSize, tmpReg3, op2Reg, negOneFld, 0, instOptions); emit->emitIns_SIMD_R_R_R(INS_pandd, typeSize, tmpReg2, tmpReg2, tmpReg3, instOptions); diff --git a/src/coreclr/jit/instr.cpp b/src/coreclr/jit/instr.cpp index 28ca9242ba3cd4..9d8cfb8c99c144 100644 --- a/src/coreclr/jit/instr.cpp +++ b/src/coreclr/jit/instr.cpp @@ -1072,7 +1072,10 @@ CodeGen::OperandDesc CodeGen::genOperandDesc(instruction ins, GenTree* op) unsigned varNum = BAD_VAR_NUM; uint16_t offset = UINT16_MAX; - if (op->isUsedFromSpillTemp()) + // A rematerializable constant that LSRA spilled and whose use consumes it directly from + // memory (GTF_NOREG_AT_USE) has no stack spill temp (see genProduceReg); it is emitted + // from its data-section home instead, handled by the constant cases below. + if (op->isUsedFromSpillTemp() && !isRematerializableConstant(op)) { assert(op->IsRegOptional()); diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index f1dd62dfcc2ae8..3d9c6be907914f 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -275,10 +275,27 @@ bool Lowering::IsSafeToContainMem(GenTree* grandparentNode, GenTree* parentNode, // interfere is due to it being address exposed. So this is the only unsafe // case. // +// In addition to interference, a constant that codegen materializes directly +// into a register without any memory access has no spill/data-section home to +// consume from, so reg-optionality is reported as unsafe for it as well (see +// IsConstantMaterializableInRegWithoutMemory). +// bool Lowering::IsSafeToMarkRegOptional(GenTree* parentNode, GenTree* childNode) const { if (!childNode->OperIs(GT_LCL_VAR)) { + // A constant that codegen materializes directly into a register without a memory + // access (e.g. zero via xorps, all-bits-set via pcmpeqd) has no data-section home. + // Marking it reg-optional would let the register allocator resolve the use to memory, + // forcing a data-section load (and a new data-section entry) that is strictly worse + // than rematerializing the value in a register. Report it as unsafe so that a + // different operand can be chosen for reg-optionality instead; such constants are + // likewise never made contained. + if (IsConstantMaterializableInRegWithoutMemory(childNode)) + { + return false; + } + // LIR edges never interfere. This includes GT_LCL_FLD, see the remarks above. return true; } @@ -295,6 +312,55 @@ bool Lowering::IsSafeToMarkRegOptional(GenTree* parentNode, GenTree* childNode) return false; } +//------------------------------------------------------------------------ +// IsConstantMaterializableInRegWithoutMemory: Determine whether a constant is +// materialized directly into a register by an instruction that does not access +// memory. +// +// Arguments: +// node - the node to check +// +// Return Value: +// True if 'node' is a floating-point, SIMD, or mask constant that codegen +// materializes with a register-only instruction (for example xorps for zero or +// pcmpeqd for all-bits-set) rather than a data-section load. +// +// Notes: +// Such constants must never be made contained or reg-optional. Otherwise the +// register allocator could resolve the use to a memory operand, forcing a +// data-section load (and a new data-section entry) that is strictly worse than +// rematerializing the value in a register. Reg-optionality is prevented via +// IsSafeToMarkRegOptional, which consults this predicate. On xarch this is exactly +// the zero and all-bits-set constants (see genSetRegToConst). Other targets always +// assign these constants a register (they are never reg-optional), so they return +// false here. +// +bool Lowering::IsConstantMaterializableInRegWithoutMemory(GenTree* node) const +{ +#if defined(TARGET_XARCH) + if (node->IsCnsFltOrDbl()) + { + return node->IsFloatPositiveZero() || node->IsFloatAllBitsSet(); + } +#if defined(FEATURE_SIMD) + if (node->IsCnsVec()) + { + // A TYP_SIMD32/TYP_SIMD64 constant only exists when the corresponding ISA + // (AVX2/AVX512) is available, so no ISA check is needed here. + return node->IsVectorZero() || node->IsVectorAllBitsSet(); + } +#endif // FEATURE_SIMD +#if defined(FEATURE_MASKED_HW_INTRINSICS) + if (node->IsCnsMsk()) + { + return node->IsMaskZero() || node->IsMaskAllBitsSet(); + } +#endif // FEATURE_MASKED_HW_INTRINSICS +#endif // TARGET_XARCH + + return false; +} + //------------------------------------------------------------------------ // LowerRange: // Lower the specified range of nodes. diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index 2926e1c2477562..3589e4189d391c 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -631,6 +631,9 @@ class Lowering final : public Phase // Check if marking an operand of a node as reg-optional is safe. bool IsSafeToMarkRegOptional(GenTree* parentNode, GenTree* node) const; + // Check if a constant is materialized directly into a register without a memory access. + bool IsConstantMaterializableInRegWithoutMemory(GenTree* node) const; + // Checks if it's profitable to optimize an shift and rotate operations to set the zero flag. bool IsProfitableToSetZeroFlag(GenTree* op) const;