From 5a4a5c44226c1e3b10f3676bc5b7c3b03d3beb3f Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Thu, 28 May 2026 10:09:07 +0200 Subject: [PATCH 1/3] JIT: Unify LowerBlockStore for init across targets Hoist the per-target `Lowering::LowerBlockStore` (which only handles InitBlk after the recent `LowerCopyBlockStore` extraction) into a shared `Lowering::LowerInitBlockStore` in `lower.cpp`, mirroring the `LowerCopyBlockStore` pattern. - Rename `LowerBlockStore` -> `LowerInitBlockStore` everywhere. - Introduce `FEATURE_HAS_ZERO_REG` in each target.h (1 for arm64, loongarch64, riscv64; 0 for amd64, x86, arm, wasm) to replace `#if defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) || ...` in the shared lowering. - Remove `BlkOpKindRepInstr` and all related dead code: - `genCodeForInitBlkRepStos` / `genCodeForCpBlkRepMovs` and the `genCodeForStoreBlk` dispatch case - LSRA: kill-set case and the two `BuildBlockStore` cases - `gentree.h` enum value and `gentree.cpp` print - x86 now takes exactly the same path as x64: previously the `too-big-to-unroll` and `not-CNS-INT` init blk cases used `rep stosb`; both now go through `LowerBlockStoreAsHelperCall` (matches the existing TODO-X86-CQ comment). - Remove the `STRESS_STORE_BLOCK_UNROLLING` stress mode (no value). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/codegen.h | 2 - src/coreclr/jit/codegenxarch.cpp | 42 ---------- src/coreclr/jit/compiler.h | 1 - src/coreclr/jit/gentree.cpp | 5 -- src/coreclr/jit/gentree.h | 3 - src/coreclr/jit/lower.cpp | 115 ++++++++++++++++++++++++++- src/coreclr/jit/lower.h | 2 +- src/coreclr/jit/lowerarmarch.cpp | 87 -------------------- src/coreclr/jit/lowerloongarch64.cpp | 64 --------------- src/coreclr/jit/lowerriscv64.cpp | 64 --------------- src/coreclr/jit/lowerwasm.cpp | 39 --------- src/coreclr/jit/lowerxarch.cpp | 107 ------------------------- src/coreclr/jit/lsrabuild.cpp | 17 ---- src/coreclr/jit/lsraxarch.cpp | 31 +------- src/coreclr/jit/targetamd64.h | 1 + src/coreclr/jit/targetarm.h | 1 + src/coreclr/jit/targetarm64.h | 1 + src/coreclr/jit/targetloongarch64.h | 1 + src/coreclr/jit/targetriscv64.h | 1 + src/coreclr/jit/targetwasm.h | 1 + src/coreclr/jit/targetx86.h | 1 + 21 files changed, 124 insertions(+), 462 deletions(-) diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index 30df97543bec50..5a933a511f1d4a 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -1175,7 +1175,6 @@ class CodeGen final : public CodeGenInterface void genCodeForReturnTrap(GenTreeOp* tree); void genCodeForStoreInd(GenTreeStoreInd* tree); void genCodeForSwap(GenTreeOp* tree); - void genCodeForCpBlkRepMovs(GenTreeBlk* cpBlkNode); void genCodeForCpBlkUnroll(GenTreeBlk* cpBlkNode); void genCodeForPhysReg(GenTreePhysReg* tree); #ifdef SWIFT_SUPPORT @@ -1252,7 +1251,6 @@ class CodeGen final : public CodeGenInterface void genCodeForStoreBlk(GenTreeBlk* storeBlkNode); void genCodeForInitBlkLoop(GenTreeBlk* initBlkNode); - void genCodeForInitBlkRepStos(GenTreeBlk* initBlkNode); void genCodeForInitBlkUnroll(GenTreeBlk* initBlkNode); unsigned genEmitJumpTable(GenTree* treeNode, bool relativeAddr); void genJumpTable(GenTree* tree); diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index 4cc36d3133fde0..a8b2e20e24a3f7 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -3047,17 +3047,6 @@ void CodeGen::genCodeForStoreBlk(GenTreeBlk* storeBlkNode) genCodeForInitBlkLoop(storeBlkNode); break; - case GenTreeBlk::BlkOpKindRepInstr: - assert(!storeBlkNode->gtBlkOpGcUnsafe); - if (isCopyBlk) - { - genCodeForCpBlkRepMovs(storeBlkNode); - } - else - { - genCodeForInitBlkRepStos(storeBlkNode); - } - break; case GenTreeBlk::BlkOpKindUnrollMemmove: case GenTreeBlk::BlkOpKindUnroll: if (isCopyBlk) @@ -3091,19 +3080,6 @@ void CodeGen::genCodeForStoreBlk(GenTreeBlk* storeBlkNode) } } -// -//------------------------------------------------------------------------ -// genCodeForInitBlkRepStos: Generate code for InitBlk using rep stos. -// -// Arguments: -// initBlkNode - The Block store for which we are generating code. -// -void CodeGen::genCodeForInitBlkRepStos(GenTreeBlk* initBlkNode) -{ - genConsumeBlockOp(initBlkNode, REG_RDI, REG_RAX, REG_RCX); - instGen(INS_r_stosb); -} - //---------------------------------------------------------------------------------- // genCodeForInitBlkUnroll: Generate unrolled block initialization code. // @@ -3681,24 +3657,6 @@ void CodeGen::genCodeForCpBlkUnroll(GenTreeBlk* node) } } -//---------------------------------------------------------------------------------- -// genCodeForCpBlkRepMovs - Generate code for CpBlk by using rep movs -// -// Arguments: -// cpBlkNode - the GT_STORE_[BLK|OBJ|DYN_BLK] -// -// Preconditions: -// The register assignments have been set appropriately. -// This is validated by genConsumeBlockOp(). -// -void CodeGen::genCodeForCpBlkRepMovs(GenTreeBlk* cpBlkNode) -{ - // Destination address goes in RDI, source address goes in RSE, and size goes in RCX. - // genConsumeBlockOp takes care of this for us. - genConsumeBlockOp(cpBlkNode, REG_RDI, REG_RSI, REG_RCX); - instGen(INS_r_movsb); -} - //------------------------------------------------------------------------ // CodeGen::genMove8IfNeeded: Conditionally move 8 bytes of a struct to the argument area // diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index e3ed2ceb179d5a..4419a66cc12bc1 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -11527,7 +11527,6 @@ class Compiler STRESS_MODE(IF_CONVERSION_COST) \ STRESS_MODE(IF_CONVERSION_INNER_LOOPS) \ STRESS_MODE(POISON_IMPLICIT_BYREFS) \ - STRESS_MODE(STORE_BLOCK_UNROLLING) \ STRESS_MODE(THREE_OPT_LAYOUT) \ STRESS_MODE(COUNT) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 3785c041d8d9de..efcafb4aa948df 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -14015,11 +14015,6 @@ void Compiler::gtDispTree(GenTree* tree, { switch (tree->AsBlk()->gtBlkOpKind) { -#ifdef TARGET_XARCH - case GenTreeBlk::BlkOpKindRepInstr: - printf(" (RepInstr)"); - break; -#endif case GenTreeBlk::BlkOpKindUnroll: printf(" (Unroll)"); break; diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 3c51fc9541e201..dd958c15268182 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -8112,9 +8112,6 @@ struct GenTreeBlk : public GenTreeIndir enum { BlkOpKindInvalid, -#ifdef TARGET_XARCH - BlkOpKindRepInstr, -#endif BlkOpKindLoop, BlkOpKindUnroll, BlkOpKindUnrollMemmove, diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index d1433f50bf2748..dbb59537cc3f13 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -10328,6 +10328,119 @@ void Lowering::LowerCopyBlockStore(GenTreeBlk* blkNode) #endif // TARGET_WASM } +//------------------------------------------------------------------------ +// LowerInitBlockStore: Shared lowering of a block init store (memset). +// +// Arguments: +// blkNode - The block store node to lower. Must be an init (not copy). +// +void Lowering::LowerInitBlockStore(GenTreeBlk* blkNode) +{ + assert(blkNode->OperIsInitBlkOp()); + +#ifdef TARGET_XARCH + TryCreateAddrMode(blkNode->Addr(), false, blkNode); +#endif + + GenTree* dstAddr = blkNode->Addr(); + GenTree* src = blkNode->Data(); + const unsigned size = blkNode->Size(); + + if (src->OperIs(GT_INIT_VAL)) + { + src->SetContained(); + src = src->AsUnOp()->gtGetOp1(); + } + +#ifdef TARGET_WASM + if (blkNode->IsZeroingGcPointersOnHeap()) + { + blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindLoop; + src->SetContained(); + } + else + { + // Use the wasm `memory.fill` instruction. + blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindNativeOpcode; + } + + if ((blkNode->gtBlkOpKind != GenTreeBlk::BlkOpKindNativeOpcode) || ((blkNode->gtFlags & GTF_IND_NONFAULTING) == 0)) + { + SetMultiplyUsed(dstAddr DEBUGARG("LowerInitBlockStore destination address")); + } +#else // !TARGET_WASM + // On xarch, SIMD stores aren't atomic at 8-byte boundaries, so we can't use the larger + // SIMD-aware unroll threshold when the destination contains GC pointers on heap. + // On arm64 SIMD stores guarantee 8-byte atomicity (and other targets don't use SIMD here). +#ifdef TARGET_XARCH + const bool canUseSimd = !blkNode->IsOnHeapAndContainsReferences(); +#else + const bool canUseSimd = true; +#endif + bool canUnroll = + src->OperIs(GT_CNS_INT) && (size <= m_compiler->getUnrollThreshold(Compiler::UnrollKind::Memset, canUseSimd)); + + if (canUnroll) + { + // The fill value of an initblk is interpreted to hold a value of (unsigned int8) + // however a constant of any size may practically reside on the evaluation stack. + // So extract the lower byte out of the initVal constant and replicate it to a larger + // constant whose size is sufficient to support the largest width store of the desired + // inline expansion. + ssize_t fill = src->AsIntCon()->IconValue() & 0xFF; + + bool useSimdFill = false; +#ifdef TARGET_XARCH + useSimdFill = canUseSimd && (size >= XMM_REGSIZE_BYTES); +#endif + + if (useSimdFill) + { + // We're going to use SIMD (and only SIMD - we don't want to occupy a GPR register + // with a fill value just to handle the remainder when we can do that with + // an overlapped SIMD load). + src->SetContained(); + } + else if (fill == 0) + { +#if FEATURE_HAS_ZERO_REG + // We can use the hardware zero register as the contained source. + src->SetContained(); +#endif + } +#ifdef TARGET_64BIT + else if (size >= REGSIZE_BYTES) + { + fill *= 0x0101010101010101LL; + src->gtType = TYP_LONG; + } +#endif + else + { + fill *= 0x01010101; + } + + blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindUnroll; + src->AsIntCon()->SetIconValue(fill); + ContainBlockStoreAddress(blkNode, size, dstAddr, nullptr); + return; + } + + if (blkNode->IsZeroingGcPointersOnHeap()) + { + blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindLoop; +#if FEATURE_HAS_ZERO_REG + // We can use the hardware zero register as the contained source. + src->SetContained(); +#endif + } + else + { + LowerBlockStoreAsHelperCall(blkNode); + } +#endif // !TARGET_WASM +} + //------------------------------------------------------------------------ // LowerBlockStoreAsHelperCall: Lower a block store node as a memset/memcpy call // @@ -12164,7 +12277,7 @@ void Lowering::LowerBlockStoreCommon(GenTreeBlk* blkNode) if (blkNode->OperIsInitBlkOp()) { - LowerBlockStore(blkNode); + LowerInitBlockStore(blkNode); } else { diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index cf4dc04a617c83..e7c59dca277742 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -436,10 +436,10 @@ class Lowering final : public Phase bool TryLowerConstIntDivOrMod(GenTree* node, GenTree** nextNode); GenTree* LowerSignedDivOrMod(GenTree* node); void LowerDivOrMod(GenTreeOp* divMod); - void LowerBlockStore(GenTreeBlk* blkNode); void LowerBlockStoreCommon(GenTreeBlk* blkNode); void LowerBlockStoreAsHelperCall(GenTreeBlk* blkNode); void LowerBlockStoreAsGcBulkCopyCall(GenTreeBlk* blkNode); + void LowerInitBlockStore(GenTreeBlk* blkNode); void LowerCopyBlockStore(GenTreeBlk* blkNode); bool TryDecomposeBlockStoreAsIndirs(GenTreeBlk* blkNode); void LowerLclHeap(GenTree* node); diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index dfe4194703495d..47b7985f974b6d 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -709,93 +709,6 @@ GenTree* Lowering::LowerBinaryArithmetic(GenTreeOp* binOp) return binOp->gtNext; } -//------------------------------------------------------------------------ -// LowerBlockStore: Lower a block store node -// -// Arguments: -// blkNode - The block store node to lower -// -void Lowering::LowerBlockStore(GenTreeBlk* blkNode) -{ - assert(blkNode->OperIsInitBlkOp()); - GenTree* dstAddr = blkNode->Addr(); - GenTree* src = blkNode->Data(); - unsigned size = blkNode->Size(); - -#ifdef DEBUG - // Use BlkOpKindLoop for more cases under stress mode - if (m_compiler->compStressCompile(Compiler::STRESS_STORE_BLOCK_UNROLLING, 50) && blkNode->OperIs(GT_STORE_BLK) && - ((blkNode->GetLayout()->GetSize() % TARGET_POINTER_SIZE) == 0) && src->IsIntegralConst(0)) - { - blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindLoop; -#ifdef TARGET_ARM64 - // On ARM64 we can just use REG_ZR instead of having to load - // the constant into a real register like on ARM32. - src->SetContained(); -#endif - return; - } -#endif - - if (src->OperIs(GT_INIT_VAL)) - { - src->SetContained(); - src = src->AsUnOp()->gtGetOp1(); - } - - if ((size <= m_compiler->getUnrollThreshold(Compiler::UnrollKind::Memset)) && src->OperIs(GT_CNS_INT)) - { - blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindUnroll; - - // The fill value of an initblk is interpreted to hold a - // value of (unsigned int8) however a constant of any size - // may practically reside on the evaluation stack. So extract - // the lower byte out of the initVal constant and replicate - // it to a larger constant whose size is sufficient to support - // the largest width store of the desired inline expansion. - - ssize_t fill = src->AsIntCon()->IconValue() & 0xFF; - - if (fill == 0) - { -#ifdef TARGET_ARM64 - // On ARM64 we can just use REG_ZR instead of having to load - // the constant into a real register like on ARM32. - src->SetContained(); -#endif - } -#ifdef TARGET_ARM64 - else if (size >= REGSIZE_BYTES) - { - fill *= 0x0101010101010101LL; - src->gtType = TYP_LONG; - } -#endif - else - { - fill *= 0x01010101; - } - - src->AsIntCon()->SetIconValue(fill); - - ContainBlockStoreAddress(blkNode, size, dstAddr, nullptr); - } - else if (blkNode->IsZeroingGcPointersOnHeap()) - { - blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindLoop; -#ifdef TARGET_ARM64 - // On ARM64 we can just use REG_ZR instead of having to load - // the constant into a real register like on ARM32. - src->SetContained(); -#endif - } - else - { - LowerBlockStoreAsHelperCall(blkNode); - return; - } -} - //------------------------------------------------------------------------ // ContainBlockStoreAddress: Attempt to contain an address used by an unrolled block store. // diff --git a/src/coreclr/jit/lowerloongarch64.cpp b/src/coreclr/jit/lowerloongarch64.cpp index 029d51f5514084..56970f4ce404d0 100644 --- a/src/coreclr/jit/lowerloongarch64.cpp +++ b/src/coreclr/jit/lowerloongarch64.cpp @@ -279,70 +279,6 @@ GenTree* Lowering::LowerStoreIndir(GenTreeStoreInd* node) return node->gtNext; } -//------------------------------------------------------------------------ -// LowerBlockStore: Set block store type -// -// Arguments: -// blkNode - The block store node of interest -// -// Return Value: -// None. -// -void Lowering::LowerBlockStore(GenTreeBlk* blkNode) -{ - assert(blkNode->OperIsInitBlkOp()); - GenTree* dstAddr = blkNode->Addr(); - GenTree* src = blkNode->Data(); - unsigned size = blkNode->Size(); - - if (src->OperIs(GT_INIT_VAL)) - { - src->SetContained(); - src = src->AsUnOp()->gtGetOp1(); - } - - if ((size <= m_compiler->getUnrollThreshold(Compiler::UnrollKind::Memset)) && src->OperIs(GT_CNS_INT)) - { - blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindUnroll; - - // The fill value of an initblk is interpreted to hold a - // value of (unsigned int8) however a constant of any size - // may practically reside on the evaluation stack. So extract - // the lower byte out of the initVal constant and replicate - // it to a larger constant whose size is sufficient to support - // the largest width store of the desired inline expansion. - - ssize_t fill = src->AsIntCon()->IconValue() & 0xFF; - if (fill == 0) - { - src->SetContained(); - } - else if (size >= REGSIZE_BYTES) - { - fill *= 0x0101010101010101LL; - src->gtType = TYP_LONG; - } - else - { - fill *= 0x01010101; - } - src->AsIntCon()->SetIconValue(fill); - - ContainBlockStoreAddress(blkNode, size, dstAddr, nullptr); - } - else if (blkNode->IsZeroingGcPointersOnHeap()) - { - blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindLoop; - // We're going to use REG_R0 for zero - src->SetContained(); - } - else - { - LowerBlockStoreAsHelperCall(blkNode); - return; - } -} - //------------------------------------------------------------------------ // ContainBlockStoreAddress: Attempt to contain an address used by an unrolled block store. // diff --git a/src/coreclr/jit/lowerriscv64.cpp b/src/coreclr/jit/lowerriscv64.cpp index d9164e5e18c7f8..e101ca52065e06 100644 --- a/src/coreclr/jit/lowerriscv64.cpp +++ b/src/coreclr/jit/lowerriscv64.cpp @@ -479,70 +479,6 @@ GenTree* Lowering::LowerStoreIndir(GenTreeStoreInd* node) return node->gtNext; } -//------------------------------------------------------------------------ -// LowerBlockStore: Set block store type -// -// Arguments: -// blkNode - The block store node of interest -// -// Return Value: -// None. -// -void Lowering::LowerBlockStore(GenTreeBlk* blkNode) -{ - assert(blkNode->OperIsInitBlkOp()); - GenTree* dstAddr = blkNode->Addr(); - GenTree* src = blkNode->Data(); - unsigned size = blkNode->Size(); - - if (src->OperIs(GT_INIT_VAL)) - { - src->SetContained(); - src = src->AsUnOp()->gtGetOp1(); - } - - if ((size <= m_compiler->getUnrollThreshold(Compiler::UnrollKind::Memset)) && src->OperIs(GT_CNS_INT)) - { - blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindUnroll; - - // The fill value of an initblk is interpreted to hold a - // value of (unsigned int8) however a constant of any size - // may practically reside on the evaluation stack. So extract - // the lower byte out of the initVal constant and replicate - // it to a larger constant whose size is sufficient to support - // the largest width store of the desired inline expansion. - - ssize_t fill = src->AsIntCon()->IconValue() & 0xFF; - if (fill == 0) - { - src->SetContained(); - } - else if (size >= REGSIZE_BYTES) - { - fill *= 0x0101010101010101LL; - src->gtType = TYP_LONG; - } - else - { - fill *= 0x01010101; - } - src->AsIntCon()->SetIconValue(fill); - - ContainBlockStoreAddress(blkNode, size, dstAddr, nullptr); - } - else if (blkNode->IsZeroingGcPointersOnHeap()) - { - blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindLoop; - // We're going to use REG_R0 for zero - src->SetContained(); - } - else - { - LowerBlockStoreAsHelperCall(blkNode); - return; - } -} - //------------------------------------------------------------------------ // ContainBlockStoreAddress: Attempt to contain an address used by an unrolled block store. // diff --git a/src/coreclr/jit/lowerwasm.cpp b/src/coreclr/jit/lowerwasm.cpp index c4f9f62ca991aa..f3c87010ef0253 100644 --- a/src/coreclr/jit/lowerwasm.cpp +++ b/src/coreclr/jit/lowerwasm.cpp @@ -287,45 +287,6 @@ void Lowering::LowerDivOrMod(GenTreeOp* divMod) ContainCheckDivOrMod(divMod); } -//------------------------------------------------------------------------ -// LowerBlockStore: Lower a block init node (memset / loop zeroing). -// The copy variant (non-InitBlkOp) is handled by the shared -// Lowering::LowerCopyBlockStore. -// -// Arguments: -// blkNode - The block store node to lower -// -void Lowering::LowerBlockStore(GenTreeBlk* blkNode) -{ - assert(blkNode->OperIsInitBlkOp()); - - GenTree* dstAddr = blkNode->Addr(); - GenTree* src = blkNode->Data(); - - if (src->OperIs(GT_INIT_VAL)) - { - src->SetContained(); - src = src->AsUnOp()->gtGetOp1(); - } - - if (blkNode->IsZeroingGcPointersOnHeap()) - { - blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindLoop; - src->SetContained(); - } - else - { - // memory.fill - blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindNativeOpcode; - } - - if (((blkNode->gtBlkOpKind != GenTreeBlk::BlkOpKindNativeOpcode) || - ((blkNode->gtFlags & GTF_IND_NONFAULTING) == 0))) - { - SetMultiplyUsed(dstAddr DEBUGARG("LowerBlockStore destination address")); - } -} - //------------------------------------------------------------------------ // LowerPutArgStk: Lower a GT_PUTARG_STK. // diff --git a/src/coreclr/jit/lowerxarch.cpp b/src/coreclr/jit/lowerxarch.cpp index a96a50d9562a0d..c92f89d560bc2d 100644 --- a/src/coreclr/jit/lowerxarch.cpp +++ b/src/coreclr/jit/lowerxarch.cpp @@ -355,113 +355,6 @@ insCflags Lowering::TruthifyingFlags(GenCondition condition) } #endif // TARGET_AMD64 -//------------------------------------------------------------------------ -// LowerBlockStore: Lower a block store node -// -// Arguments: -// blkNode - The block store node to lower -// -void Lowering::LowerBlockStore(GenTreeBlk* blkNode) -{ - assert(blkNode->OperIsInitBlkOp()); - TryCreateAddrMode(blkNode->Addr(), false, blkNode); - - GenTree* dstAddr = blkNode->Addr(); - GenTree* src = blkNode->Data(); - unsigned size = blkNode->Size(); - -#ifdef DEBUG - // Use BlkOpKindLoop for more cases under stress mode - if (m_compiler->compStressCompile(Compiler::STRESS_STORE_BLOCK_UNROLLING, 50) && blkNode->OperIs(GT_STORE_BLK) && - ((blkNode->GetLayout()->GetSize() % TARGET_POINTER_SIZE) == 0) && src->IsIntegralConst(0)) - { - blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindLoop; - return; - } -#endif - - if (src->OperIs(GT_INIT_VAL)) - { - src->SetContained(); - src = src->AsUnOp()->gtGetOp1(); - } - - if (size <= m_compiler->getUnrollThreshold(Compiler::UnrollKind::Memset)) - { - if (!src->OperIs(GT_CNS_INT)) - { - // TODO-CQ: We could unroll even when the initialization value is not a constant - // by inserting a MUL init, 0x01010101 instruction. We need to determine if the - // extra latency that MUL introduces isn't worse that rep stosb. Likely not. - blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindRepInstr; - } - else - { - // The fill value of an initblk is interpreted to hold a - // value of (unsigned int8) however a constant of any size - // may practically reside on the evaluation stack. So extract - // the lower byte out of the initVal constant and replicate - // it to a larger constant whose size is sufficient to support - // the largest width store of the desired inline expansion. - - ssize_t fill = src->AsIntCon()->IconValue() & 0xFF; - - const bool canUseSimd = !blkNode->IsOnHeapAndContainsReferences(); - if (size > m_compiler->getUnrollThreshold(Compiler::UnrollKind::Memset, canUseSimd)) - { - // It turns out we can't use SIMD so the default threshold is too big - goto TOO_BIG_TO_UNROLL; - } - if (canUseSimd && (size >= XMM_REGSIZE_BYTES)) - { - // We're going to use SIMD (and only SIMD - we don't want to occupy a GPR register - // with a fill value just to handle the remainder when we can do that with - // an overlapped SIMD load). - src->SetContained(); - } - else if (fill == 0) - { - // Leave as is - zero shouldn't be contained when we don't use SIMD. - } -#ifdef TARGET_AMD64 - else if (size >= REGSIZE_BYTES) - { - fill *= 0x0101010101010101LL; - src->gtType = TYP_LONG; - } -#endif - else - { - fill *= 0x01010101; - } - - blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindUnroll; - src->AsIntCon()->SetIconValue(fill); - - ContainBlockStoreAddress(blkNode, size, dstAddr, nullptr); - } - } - else - { - TOO_BIG_TO_UNROLL: - if (blkNode->IsZeroingGcPointersOnHeap()) - { - blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindLoop; - } - else - { -#ifdef TARGET_AMD64 - LowerBlockStoreAsHelperCall(blkNode); - return; -#else - // TODO-X86-CQ: Investigate whether a helper call would be beneficial on x86 - blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindRepInstr; -#endif - } - } - assert(blkNode->gtBlkOpKind != GenTreeBlk::BlkOpKindInvalid); -} - //------------------------------------------------------------------------ // ContainBlockStoreAddress: Attempt to contain an address used by an unrolled block store. // diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 8e69cdb7d7102e..eaeb0aeb31a7c1 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -873,23 +873,6 @@ regMaskTP LinearScan::getKillSetForBlockStore(GenTreeBlk* blkNode) bool isCopyBlk = varTypeIsStruct(blkNode->Data()); switch (blkNode->gtBlkOpKind) { -#ifdef TARGET_XARCH - case GenTreeBlk::BlkOpKindRepInstr: - if (isCopyBlk) - { - // rep movs kills RCX, RDI and RSI - killMask.AddGprRegs(SRBM_RCX | SRBM_RDI | SRBM_RSI DEBUG_ARG(RBM_ALLINT)); - } - else - { - // rep stos kills RCX and RDI. - // (Note that the Data() node, if not constant, will be assigned to - // RCX, but it's find that this kills it, as the value is not available - // after this node in any case.) - killMask.AddGprRegs(SRBM_RDI | SRBM_RCX DEBUG_ARG(RBM_ALLINT)); - } - break; -#endif case GenTreeBlk::BlkOpKindUnrollMemmove: case GenTreeBlk::BlkOpKindUnroll: case GenTreeBlk::BlkOpKindLoop: diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp index c373999d3544ce..53082fd86658cf 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -1406,9 +1406,7 @@ int LinearScan::BuildBlockStore(GenTreeBlk* blkNode) GenTree* srcAddrOrFill = nullptr; - SingleTypeRegSet dstAddrRegMask = RBM_NONE; - SingleTypeRegSet srcRegMask = RBM_NONE; - SingleTypeRegSet sizeRegMask = RBM_NONE; + SingleTypeRegSet srcRegMask = RBM_NONE; RefPosition* internalIntDef = nullptr; #ifdef TARGET_X86 @@ -1470,12 +1468,6 @@ int LinearScan::BuildBlockStore(GenTreeBlk* blkNode) } break; - case GenTreeBlk::BlkOpKindRepInstr: - dstAddrRegMask = SRBM_RDI; - srcRegMask = SRBM_RAX; - sizeRegMask = SRBM_RCX; - break; - case GenTreeBlk::BlkOpKindLoop: // Needed for offsetReg buildInternalIntRegisterDefForNode(blkNode, availableIntRegs); @@ -1584,28 +1576,9 @@ int LinearScan::BuildBlockStore(GenTreeBlk* blkNode) } break; - case GenTreeBlk::BlkOpKindRepInstr: - dstAddrRegMask = SRBM_RDI; - srcRegMask = SRBM_RSI; - sizeRegMask = SRBM_RCX; - break; - default: unreached(); } - - if ((srcAddrOrFill == nullptr) && (srcRegMask != RBM_NONE)) - { - // This is a local source; we'll use a temp register for its address. - assert(src->isContained() && src->OperIs(GT_LCL_VAR, GT_LCL_FLD)); - buildInternalIntRegisterDefForNode(blkNode, srcRegMask); - } - } - - if (sizeRegMask != RBM_NONE) - { - // Reserve a temp register for the block size argument. - buildInternalIntRegisterDefForNode(blkNode, sizeRegMask); } int useCount = 0; @@ -1613,7 +1586,7 @@ int LinearScan::BuildBlockStore(GenTreeBlk* blkNode) if (!dstAddr->isContained()) { useCount++; - BuildUse(dstAddr, ForceLowGprForApxIfNeeded(dstAddr, dstAddrRegMask, getEvexIsSupported())); + BuildUse(dstAddr, ForceLowGprForApxIfNeeded(dstAddr, RBM_NONE, getEvexIsSupported())); } else if (dstAddr->OperIsAddrMode()) { diff --git a/src/coreclr/jit/targetamd64.h b/src/coreclr/jit/targetamd64.h index 55c0dae697d7b6..6b89eba0ea30fd 100644 --- a/src/coreclr/jit/targetamd64.h +++ b/src/coreclr/jit/targetamd64.h @@ -61,6 +61,7 @@ // MAX_MULTIREG_COUNT - 1. #endif // !UNIX_AMD64_ABI +#define FEATURE_HAS_ZERO_REG 0 // Target does not have a hardware "zero register" usable as a containable source #define NOGC_WRITE_BARRIERS 0 // We DO-NOT have specialized WriteBarrier JIT Helpers that DO-NOT trash the RBM_CALLEE_TRASH registers #define USER_ARGS_COME_LAST 1 #define TARGET_POINTER_SIZE 8 // equal to sizeof(void*) and the managed pointer size in bytes for this target diff --git a/src/coreclr/jit/targetarm.h b/src/coreclr/jit/targetarm.h index 7101102f31d9ec..acd783c41f4d89 100644 --- a/src/coreclr/jit/targetarm.h +++ b/src/coreclr/jit/targetarm.h @@ -24,6 +24,7 @@ #define FEATURE_MULTIREG_ARGS_OR_RET 1 // Support for passing and/or returning single values in more than one register (including HFA support) #define FEATURE_MULTIREG_ARGS 1 // Support for passing a single argument in more than one register (including passing HFAs) #define FEATURE_MULTIREG_RET 1 // Support for returning a single value in more than one register (including HFA returns) +#define FEATURE_HAS_ZERO_REG 0 // Target does not have a hardware "zero register" usable as a containable source #define MAX_PASS_SINGLEREG_BYTES 8 // Maximum size of a struct passed in a single register (double). #define MAX_PASS_MULTIREG_BYTES 32 // Maximum size of a struct that could be passed in more than one register (Max is an HFA of 4 doubles) #define MAX_RET_MULTIREG_BYTES 32 // Maximum size of a struct that could be returned in more than one register (Max is an HFA of 4 doubles) diff --git a/src/coreclr/jit/targetarm64.h b/src/coreclr/jit/targetarm64.h index 1102fc888a9533..3cb14d537f39ca 100644 --- a/src/coreclr/jit/targetarm64.h +++ b/src/coreclr/jit/targetarm64.h @@ -26,6 +26,7 @@ #define FEATURE_MULTIREG_ARGS_OR_RET 1 // Support for passing and/or returning single values in more than one register #define FEATURE_MULTIREG_ARGS 1 // Support for passing a single argument in more than one register #define FEATURE_MULTIREG_RET 1 // Support for returning a single value in more than one register +#define FEATURE_HAS_ZERO_REG 1 // Target has a hardware "zero register" (e.g. REG_ZR on ARM64) usable as a containable source for zero stores #define MAX_PASS_SINGLEREG_BYTES 16 // Maximum size of a struct passed in a single register (16-byte vector). #define MAX_PASS_MULTIREG_BYTES 64 // Maximum size of a struct that could be passed in more than one register (max is 4 16-byte vectors using an HVA) #define MAX_RET_MULTIREG_BYTES 64 // Maximum size of a struct that could be returned in more than one register (Max is an HVA of 4 16-byte vectors) diff --git a/src/coreclr/jit/targetloongarch64.h b/src/coreclr/jit/targetloongarch64.h index a130a9cc2c3f68..468abc8dbc6dd9 100644 --- a/src/coreclr/jit/targetloongarch64.h +++ b/src/coreclr/jit/targetloongarch64.h @@ -32,6 +32,7 @@ #define FEATURE_MULTIREG_ARGS_OR_RET 1 // Support for passing and/or returning single values in more than one register #define FEATURE_MULTIREG_ARGS 1 // Support for passing a single argument in more than one register #define FEATURE_MULTIREG_RET 1 // Support for returning a single value in more than one register +#define FEATURE_HAS_ZERO_REG 1 // Target has a hardware "zero register" (e.g. REG_R0/$zero) usable as a containable source for zero stores #define MAX_PASS_SINGLEREG_BYTES 8 // Maximum size of a struct passed in a single register (8-byte). #define MAX_PASS_MULTIREG_BYTES 16 // Maximum size of a struct that could be passed in more than one register #define MAX_RET_MULTIREG_BYTES 16 // Maximum size of a struct that could be returned in more than one register (Max is an HFA of 2 doubles) diff --git a/src/coreclr/jit/targetriscv64.h b/src/coreclr/jit/targetriscv64.h index d81b12b947cc31..a17b95ecfcd124 100644 --- a/src/coreclr/jit/targetriscv64.h +++ b/src/coreclr/jit/targetriscv64.h @@ -27,6 +27,7 @@ #define FEATURE_MULTIREG_ARGS_OR_RET 1 // Support for passing and/or returning single values in more than one register #define FEATURE_MULTIREG_ARGS 1 // Support for passing a single argument in more than one register #define FEATURE_MULTIREG_RET 1 // Support for returning a single value in more than one register +#define FEATURE_HAS_ZERO_REG 1 // Target has a hardware "zero register" (e.g. REG_R0/x0) usable as a containable source for zero stores #define MAX_PASS_SINGLEREG_BYTES 8 // Maximum size of a struct passed in a single register (8-byte vector). #define MAX_PASS_MULTIREG_BYTES 16 // Maximum size of a struct that could be passed in more than one register #define MAX_RET_MULTIREG_BYTES 16 // Maximum size of a struct that could be returned in more than one register (Max is an HFA or 2 doubles) diff --git a/src/coreclr/jit/targetwasm.h b/src/coreclr/jit/targetwasm.h index 0f2f7e24be83e7..0f1ce3af46ddc3 100644 --- a/src/coreclr/jit/targetwasm.h +++ b/src/coreclr/jit/targetwasm.h @@ -29,6 +29,7 @@ #define FEATURE_MULTIREG_ARGS_OR_RET 0 // Support for passing and/or returning single values in more than one register #define FEATURE_MULTIREG_ARGS 0 // Support for passing a single argument in more than one register #define FEATURE_MULTIREG_RET 0 // Support for returning a single value in more than one register +#define FEATURE_HAS_ZERO_REG 0 // Target does not have a hardware "zero register" usable as a containable source #define MAX_PASS_SINGLEREG_BYTES 8 // Maximum size of a struct passed in a single register (long/double). #define MAX_PASS_MULTIREG_BYTES 0 // Maximum size of a struct that could be passed in more than one register #define MAX_RET_MULTIREG_BYTES 0 // Maximum size of a struct that could be returned in more than one register (Max is an HFA or 2 doubles) diff --git a/src/coreclr/jit/targetx86.h b/src/coreclr/jit/targetx86.h index 8fc6001a129a08..dd338c316a3d0d 100644 --- a/src/coreclr/jit/targetx86.h +++ b/src/coreclr/jit/targetx86.h @@ -30,6 +30,7 @@ #define FEATURE_MULTIREG_ARGS_OR_RET 1 // Support for passing and/or returning single values in more than one register #define FEATURE_MULTIREG_ARGS 0 // Support for passing a single argument in more than one register #define FEATURE_MULTIREG_RET 1 // Support for returning a single value in more than one register +#define FEATURE_HAS_ZERO_REG 0 // Target does not have a hardware "zero register" usable as a containable source #define MAX_PASS_SINGLEREG_BYTES 8 // Maximum size of a struct passed in a single register (double). #define MAX_PASS_MULTIREG_BYTES 0 // No multireg arguments #define MAX_RET_MULTIREG_BYTES 8 // Maximum size of a struct that could be returned in more than one register From b27f550c797a0477150656ce80da95e9c0dfe9e1 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Thu, 28 May 2026 16:09:48 +0200 Subject: [PATCH 2/3] JIT: Simplify getKillSetForBlockStore after RepInstr removal Address Copilot PR review feedback: 'isCopyBlk' was unused after removing the BlkOpKindRepInstr case, and the remaining switch only had no-op cases. All current block-store kinds (Unroll/UnrollMemmove/Loop/Invalid) leave the kill set as RBM_NONE, so collapse the function body to a direct return and document why no kill set is needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/lsrabuild.cpp | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index eaeb0aeb31a7c1..4571e903b4215f 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -868,20 +868,11 @@ regMaskTP LinearScan::getKillSetForCall(GenTreeCall* call) regMaskTP LinearScan::getKillSetForBlockStore(GenTreeBlk* blkNode) { assert(blkNode->OperIsStoreBlk()); - regMaskTP killMask = RBM_NONE; - - bool isCopyBlk = varTypeIsStruct(blkNode->Data()); - switch (blkNode->gtBlkOpKind) - { - case GenTreeBlk::BlkOpKindUnrollMemmove: - case GenTreeBlk::BlkOpKindUnroll: - case GenTreeBlk::BlkOpKindLoop: - case GenTreeBlk::BlkOpKindInvalid: - // for these 'gtBlkOpKind' kinds, we leave 'killMask' = RBM_NONE - break; - } - return killMask; + // After removing BlkOpKindRepInstr, the remaining block-store kinds + // (BlkOpKindUnroll, BlkOpKindUnrollMemmove, BlkOpKindLoop, BlkOpKindInvalid) + // don't kill any fixed registers; helper calls handle their own kill sets. + return RBM_NONE; } #ifdef FEATURE_HW_INTRINSICS From 0750b8bbbca42f2add3cb00776df34b6b67691a7 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Thu, 28 May 2026 16:29:40 +0200 Subject: [PATCH 3/3] clean up --- src/coreclr/jit/lower.cpp | 39 +++++++---------------------------- src/coreclr/jit/lowerwasm.cpp | 38 ++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 31 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index dbb59537cc3f13..9ae4e510346d35 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -10328,6 +10328,7 @@ void Lowering::LowerCopyBlockStore(GenTreeBlk* blkNode) #endif // TARGET_WASM } +#ifndef TARGET_WASM //------------------------------------------------------------------------ // LowerInitBlockStore: Shared lowering of a block init store (memset). // @@ -10352,44 +10353,20 @@ void Lowering::LowerInitBlockStore(GenTreeBlk* blkNode) src = src->AsUnOp()->gtGetOp1(); } -#ifdef TARGET_WASM - if (blkNode->IsZeroingGcPointersOnHeap()) - { - blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindLoop; - src->SetContained(); - } - else - { - // Use the wasm `memory.fill` instruction. - blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindNativeOpcode; - } - - if ((blkNode->gtBlkOpKind != GenTreeBlk::BlkOpKindNativeOpcode) || ((blkNode->gtFlags & GTF_IND_NONFAULTING) == 0)) - { - SetMultiplyUsed(dstAddr DEBUGARG("LowerInitBlockStore destination address")); - } -#else // !TARGET_WASM - // On xarch, SIMD stores aren't atomic at 8-byte boundaries, so we can't use the larger - // SIMD-aware unroll threshold when the destination contains GC pointers on heap. - // On arm64 SIMD stores guarantee 8-byte atomicity (and other targets don't use SIMD here). -#ifdef TARGET_XARCH - const bool canUseSimd = !blkNode->IsOnHeapAndContainsReferences(); -#else +#ifdef TARGET_ARM64 + // On arm64 SIMD stores guarantee 8-byte atomicity when data is 8-byte aligned const bool canUseSimd = true; +#else + const bool canUseSimd = !blkNode->IsOnHeapAndContainsReferences(); #endif - bool canUnroll = - src->OperIs(GT_CNS_INT) && (size <= m_compiler->getUnrollThreshold(Compiler::UnrollKind::Memset, canUseSimd)); - if (canUnroll) + if (src->OperIs(GT_CNS_INT) && (size <= m_compiler->getUnrollThreshold(Compiler::UnrollKind::Memset, canUseSimd))) { // The fill value of an initblk is interpreted to hold a value of (unsigned int8) - // however a constant of any size may practically reside on the evaluation stack. - // So extract the lower byte out of the initVal constant and replicate it to a larger - // constant whose size is sufficient to support the largest width store of the desired - // inline expansion. ssize_t fill = src->AsIntCon()->IconValue() & 0xFF; bool useSimdFill = false; + #ifdef TARGET_XARCH useSimdFill = canUseSimd && (size >= XMM_REGSIZE_BYTES); #endif @@ -10438,8 +10415,8 @@ void Lowering::LowerInitBlockStore(GenTreeBlk* blkNode) { LowerBlockStoreAsHelperCall(blkNode); } -#endif // !TARGET_WASM } +#endif // !TARGET_WASM //------------------------------------------------------------------------ // LowerBlockStoreAsHelperCall: Lower a block store node as a memset/memcpy call diff --git a/src/coreclr/jit/lowerwasm.cpp b/src/coreclr/jit/lowerwasm.cpp index f3c87010ef0253..750fd6bed58612 100644 --- a/src/coreclr/jit/lowerwasm.cpp +++ b/src/coreclr/jit/lowerwasm.cpp @@ -287,6 +287,44 @@ void Lowering::LowerDivOrMod(GenTreeOp* divMod) ContainCheckDivOrMod(divMod); } +//------------------------------------------------------------------------ +// LowerInitBlockStore: Lower a block init node (memset / loop zeroing) for WASM. +// The copy variant (non-InitBlkOp) is handled by the shared +// Lowering::LowerCopyBlockStore. +// +// Arguments: +// blkNode - The block store node to lower +// +void Lowering::LowerInitBlockStore(GenTreeBlk* blkNode) +{ + assert(blkNode->OperIsInitBlkOp()); + + GenTree* dstAddr = blkNode->Addr(); + GenTree* src = blkNode->Data(); + + if (src->OperIs(GT_INIT_VAL)) + { + src->SetContained(); + src = src->AsUnOp()->gtGetOp1(); + } + + if (blkNode->IsZeroingGcPointersOnHeap()) + { + blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindLoop; + src->SetContained(); + } + else + { + // Use the wasm `memory.fill` instruction. + blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindNativeOpcode; + } + + if ((blkNode->gtBlkOpKind != GenTreeBlk::BlkOpKindNativeOpcode) || ((blkNode->gtFlags & GTF_IND_NONFAULTING) == 0)) + { + SetMultiplyUsed(dstAddr DEBUGARG("LowerInitBlockStore destination address")); + } +} + //------------------------------------------------------------------------ // LowerPutArgStk: Lower a GT_PUTARG_STK. //