From de954cff2e95a7e9e1e53f4a308e9fd5c8a87ebe Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 9 Jul 2026 20:15:53 -0700 Subject: [PATCH 1/8] Recognize bit-manipulation idioms and emit bzhi/bts/btc/btr on xarch Lowers several single-bit and bit-field idioms to dedicated x86 instructions: x & ((1 << y) - 1) -> bzhi (reuses NI_AVX2_ZeroHighBits) x | (1 << y) -> bts (GT_BIT_SET) x & ~(1 << y) -> btr (GT_BIT_CLEAR) x ^ (1 << y) -> btc (GT_BIT_INVERT) bzhi is recognized in LowerBinaryArithmetic and folded into the existing AVX2 scalar intrinsic, so containment and memory operands work for free. It matches both the ADD(-1) and SUB(1) mask forms and only fires for a variable shift amount, since a constant amount folds to a plain and with an immediate. The bts/btr/btc patterns reuse the existing GT_BIT_SET/GT_BIT_CLEAR/GT_BIT_INVERT opers (shared with riscv64) with xarch codegen, LSRA, and emitter support. Only the reg,reg form is used: the value operand is kept in a register so the implicit masking of the bit index (mod the operand width) matches the shl semantics, and because the memory-destination forms are slower. They likewise only fire for a variable bit index, since a constant index folds to a plain or/and/xor with an immediate. bextr (constant-control field extract) is intentionally left out: it must materialize the control word and is multi-uop, so shr/sar + and in place is smaller and faster; the fully-variable (x >> y) & ((1 << z) - 1) case already lowers optimally to shrx + bzhi. This can be revisited with a cost/liveness-aware heuristic in a follow-up. Contributes to #81514 Fixes #81512 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/codegen.h | 1 + src/coreclr/jit/codegenxarch.cpp | 43 ++ src/coreclr/jit/emitxarch.cpp | 9 + src/coreclr/jit/gtlist.h | 10 +- src/coreclr/jit/instrsxarch.h | 6 + src/coreclr/jit/lower.h | 2 + src/coreclr/jit/lowerxarch.cpp | 383 ++++++++++++++++-- src/coreclr/jit/lsraxarch.cpp | 3 + .../JIT/opt/InstructionCombining/Bzhi.cs | 46 +++ .../JIT/opt/InstructionCombining/Bzhi.csproj | 15 + .../JIT/opt/InstructionCombining/SingleBit.cs | 76 +++- .../opt/InstructionCombining/SingleBit.csproj | 4 +- 12 files changed, 560 insertions(+), 38 deletions(-) create mode 100644 src/tests/JIT/opt/InstructionCombining/Bzhi.cs create mode 100644 src/tests/JIT/opt/InstructionCombining/Bzhi.csproj diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index 1fc303b4162b3d..8459a7f918a374 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -805,6 +805,7 @@ class CodeGen final : public CodeGenInterface void genCodeForDivMod(GenTreeOp* treeNode); void genCodeForMul(GenTreeOp* treeNode); + void genCodeForBitOp(GenTreeOp* treeNode); void genCodeForIncSaturate(GenTree* treeNode); void genCodeForMulHi(GenTreeOp* treeNode); void genLeaInstruction(GenTreeAddrMode* lea); diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index 5308e680efa374..74de7a53977a07 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -1129,6 +1129,43 @@ void CodeGen::genCodeForBinary(GenTreeOp* treeNode) genProduceReg(treeNode); } +//------------------------------------------------------------------------ +// genCodeForBitOp: Generate code for a GT_BIT_SET/GT_BIT_CLEAR/GT_BIT_INVERT operation, i.e. the +// value-producing `bts`/`btr`/`btc` instructions which set, reset, or complement a single bit of op1 +// selected by op2. +// +// Arguments: +// treeNode - the node to generate the code for +// +void CodeGen::genCodeForBitOp(GenTreeOp* treeNode) +{ + assert(treeNode->OperIs(GT_BIT_SET, GT_BIT_CLEAR, GT_BIT_INVERT)); + + GenTree* op1 = treeNode->gtGetOp1(); // value (read-modify-write destination) + GenTree* op2 = treeNode->gtGetOp2(); // bit index + + genConsumeOperands(treeNode); + + regNumber targetReg = treeNode->GetRegNum(); + var_types targetType = genActualType(treeNode); + emitAttr size = emitTypeSize(targetType); + emitter* emit = GetEmitter(); + + assert((targetType == TYP_INT) || (targetType == TYP_LONG)); + assert(op1->isUsedFromReg() && op2->isUsedFromReg()); + + instruction ins = treeNode->OperIs(GT_BIT_SET) ? INS_bts : treeNode->OperIs(GT_BIT_CLEAR) ? INS_btr : INS_btc; + + // These are read-modify-write: the destination register also supplies the value operand. + inst_Mov(targetType, targetReg, op1->GetRegNum(), /* canSkip */ true); + + // The BT-family reg,reg encoding places the destination in the r/m slot and the bit index in + // the reg slot, so the operands are passed reversed (see the note in instrsxarch.h). + emit->emitIns_R_R(ins, size, op2->GetRegNum(), targetReg); + + genProduceReg(treeNode); +} + //------------------------------------------------------------------------ // genCodeForMul: Generate code for a MUL operation. // @@ -1892,6 +1929,12 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) genCodeForBinary(treeNode->AsOp()); break; + case GT_BIT_SET: + case GT_BIT_CLEAR: + case GT_BIT_INVERT: + genCodeForBitOp(treeNode->AsOp()); + break; + case GT_MUL: if (varTypeIsFloating(treeNode->TypeGet())) { diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp index 2d8b9328edcbb8..ad9efca732d8e5 100644 --- a/src/coreclr/jit/emitxarch.cpp +++ b/src/coreclr/jit/emitxarch.cpp @@ -13495,6 +13495,15 @@ void emitter::emitDispIns( break; } + if ((ins == INS_bts) || (ins == INS_btr) || (ins == INS_btc)) + { + // The BT-family reg,reg encoding stores its operands reversed. Display them in + // the normal `dest, index` order. + printf("%s", emitRegName(id->idReg2(), tgtAttr)); + printf(", %s", emitRegName(id->idReg1(), srcAttr)); + break; + } + printf("%s", emitRegName(id->idReg1(), tgtAttr)); printf(", %s", emitRegName(id->idReg2(), srcAttr)); break; diff --git a/src/coreclr/jit/gtlist.h b/src/coreclr/jit/gtlist.h index be81ca9f22052d..e43ac686e1a9d5 100644 --- a/src/coreclr/jit/gtlist.h +++ b/src/coreclr/jit/gtlist.h @@ -298,14 +298,16 @@ GTNODE(SH3ADD_UW , GenTreeOp ,0,0,GTK_BINOP|DBK_NOTHIR) GTNODE(ADD_UW , GenTreeOp ,0,0,GTK_BINOP|DBK_NOTHIR) // Maps to riscv64 slli.uw instruction. Computes result = zext(op1[31..0]) << imm. GTNODE(SLLI_UW , GenTreeOp ,0,0,GTK_BINOP|DBK_NOTHIR) +#endif // TARGET_RISCV64 -// Maps to bset/bseti instruction. Computes result = op1 | (1 << op2) +#if defined(TARGET_RISCV64) || defined(TARGET_XARCH) +// Maps to riscv64 bset/bseti and xarch bts. Computes result = op1 | (1 << op2) GTNODE(BIT_SET , GenTreeOp ,0,0,GTK_BINOP|DBK_NOTHIR) -// Maps to bclr/bclri instruction. Computes result = op1 & ~(1 << op2) +// Maps to riscv64 bclr/bclri and xarch btr. Computes result = op1 & ~(1 << op2) GTNODE(BIT_CLEAR , GenTreeOp ,0,0,GTK_BINOP|DBK_NOTHIR) -// Maps to binv/binvi instruction. Computes result = op1 ^ (1 << op2) +// Maps to riscv64 binv/binvi and xarch btc. Computes result = op1 ^ (1 << op2) GTNODE(BIT_INVERT , GenTreeOp ,0,0,GTK_BINOP|DBK_NOTHIR) -#endif +#endif // TARGET_RISCV64 || TARGET_XARCH //----------------------------------------------------------------------------- // Other nodes that look like unary/binary operators: diff --git a/src/coreclr/jit/instrsxarch.h b/src/coreclr/jit/instrsxarch.h index 959aab27c0a5f9..b7232c66573278 100644 --- a/src/coreclr/jit/instrsxarch.h +++ b/src/coreclr/jit/instrsxarch.h @@ -95,6 +95,12 @@ INST4(lea, "lea", IUM_WR, BAD_CODE, BAD_CODE, // and the registers need to be reversed to get the correct encoding. INST3(bt, "bt", IUM_RD, 0x0F00A3, BAD_CODE, 0x0F00A3, 1C, 2X, INS_TT_NONE, Undefined_OF | Undefined_SF | Undefined_ZF | Undefined_AF | Undefined_PF | Writes_CF | Encoding_REX2) +// BTS/BTR/BTC are only emitted in their reg,reg form (like BT the registers are reversed to get +// the correct encoding). They read+write the first operand and write the old bit value to CF. +INST3(bts, "bts", IUM_RW, 0x0F00AB, BAD_CODE, 0x0F00AB, 1C, 2X, INS_TT_NONE, Undefined_OF | Undefined_SF | Undefined_ZF | Undefined_AF | Undefined_PF | Writes_CF | Encoding_REX2) +INST3(btr, "btr", IUM_RW, 0x0F00B3, BAD_CODE, 0x0F00B3, 1C, 2X, INS_TT_NONE, Undefined_OF | Undefined_SF | Undefined_ZF | Undefined_AF | Undefined_PF | Writes_CF | Encoding_REX2) +INST3(btc, "btc", IUM_RW, 0x0F00BB, BAD_CODE, 0x0F00BB, 1C, 2X, INS_TT_NONE, Undefined_OF | Undefined_SF | Undefined_ZF | Undefined_AF | Undefined_PF | Writes_CF | Encoding_REX2) + INST3(bsr, "bsr", IUM_WR, BAD_CODE, BAD_CODE, 0x0F00BD, 3C, 1C, INS_TT_NONE, Undefined_OF | Undefined_SF | Writes_ZF | Undefined_AF | Undefined_PF | Undefined_CF | Encoding_REX2) INST3(bsf, "bsf", IUM_WR, BAD_CODE, BAD_CODE, 0x0F00BC, 3C, 1C, INS_TT_NONE, Undefined_OF | Undefined_SF | Writes_ZF | Undefined_AF | Undefined_PF | Undefined_CF | Encoding_REX2) diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index b8435548fd8cf0..f70806c562b4a1 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -509,7 +509,9 @@ class Lowering final : public Phase GenTree* TryLowerAndOpToResetLowestSetBit(GenTreeOp* andNode); GenTree* TryLowerAndOpToExtractLowestSetBit(GenTreeOp* andNode); GenTree* TryLowerAndOpToAndNot(GenTreeOp* andNode); + GenTree* TryLowerAndOpToZeroHighBits(GenTreeOp* andNode); GenTree* TryLowerXorOpToGetMaskUpToLowestSetBit(GenTreeOp* xorNode); + GenTree* TryLowerBitwiseOpToBitOp(GenTreeOp* binOp); void LowerBswapOp(GenTreeOp* node); GenTree* LowerHWIntrinsicDotInnerMulSum(GenTreeHWIntrinsic* node); #elif defined(TARGET_ARM64) diff --git a/src/coreclr/jit/lowerxarch.cpp b/src/coreclr/jit/lowerxarch.cpp index 0359b806c6e1bc..f16312fcf73a05 100644 --- a/src/coreclr/jit/lowerxarch.cpp +++ b/src/coreclr/jit/lowerxarch.cpp @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. /*XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX @@ -260,6 +260,15 @@ GenTree* Lowering::LowerMul(GenTreeOp* mul) // GenTree* Lowering::LowerBinaryArithmetic(GenTreeOp* binOp) { + if (m_compiler->opts.OptimizationEnabled() && varTypeIsIntegral(binOp) && binOp->OperIs(GT_OR, GT_XOR, GT_AND)) + { + GenTree* replacementNode = TryLowerBitwiseOpToBitOp(binOp); + if (replacementNode != nullptr) + { + return replacementNode->gtNext; + } + } + #ifdef FEATURE_HW_INTRINSICS if (m_compiler->opts.OptimizationEnabled() && varTypeIsIntegral(binOp)) { @@ -282,6 +291,12 @@ GenTree* Lowering::LowerBinaryArithmetic(GenTreeOp* binOp) { return replacementNode->gtNext; } + + replacementNode = TryLowerAndOpToZeroHighBits(binOp); + if (replacementNode != nullptr) + { + return replacementNode->gtNext; + } } else if (binOp->OperIs(GT_XOR)) { @@ -6156,7 +6171,8 @@ bool Lowering::TryInvertMask(GenTree* node, unsigned simdSize, var_types simdBas } //---------------------------------------------------------------------------------------------- -// Lowering::TryLowerAndOpToResetLowestSetBit: Lowers a tree AND(X, ADD(X, -1)) to HWIntrinsic::ResetLowestSetBit +// Lowering::TryLowerAndOpToResetLowestSetBit: Lowers a tree AND(X, ADD(X, -1)) (or the equivalent +// AND(X, SUB(X, 1))) to HWIntrinsic::ResetLowestSetBit // // Arguments: // andNode - GT_AND node of integral type @@ -6177,13 +6193,26 @@ GenTree* Lowering::TryLowerAndOpToResetLowestSetBit(GenTreeOp* andNode) } GenTree* op2 = andNode->gtGetOp2(); - if (!op2->OperIs(GT_ADD)) + + // op2 must be ADD(X, -1), or the equivalent, un-canonicalized SUB(X, 1). Global morph normally + // rewrites the subtraction into the addition form, but that only happens during global morph so + // a SUB introduced afterwards can still reach here. + ssize_t expectedConst; + if (op2->OperIs(GT_ADD)) + { + expectedConst = -1; + } + else if (op2->OperIs(GT_SUB)) + { + expectedConst = 1; + } + else { return nullptr; } GenTree* addOp2 = op2->gtGetOp2(); - if (!addOp2->IsIntegralConst(-1)) + if (!addOp2->IsIntegralConst(expectedConst)) { return nullptr; } @@ -6201,18 +6230,23 @@ GenTree* Lowering::TryLowerAndOpToResetLowestSetBit(GenTreeOp* andNode) return nullptr; } - NamedIntrinsic intrinsic; - if (op1->TypeIs(TYP_LONG) && m_compiler->compOpportunisticallyDependsOn(InstructionSet_AVX2_X64)) + if (!m_compiler->compOpportunisticallyDependsOn(InstructionSet_AVX2)) { - intrinsic = NamedIntrinsic::NI_AVX2_X64_ResetLowestSetBit; + return nullptr; } - else if (m_compiler->compOpportunisticallyDependsOn(InstructionSet_AVX2)) + + NamedIntrinsic intrinsic; +#if defined(TARGET_AMD64) + if (andNode->TypeIs(TYP_LONG)) { - intrinsic = NamedIntrinsic::NI_AVX2_ResetLowestSetBit; + intrinsic = NamedIntrinsic::NI_AVX2_X64_ResetLowestSetBit; } else +#endif // TARGET_AMD64 { - return nullptr; + // On x86 longs are decomposed before lowering, so only the 32-bit form is reachable here. + assert(andNode->TypeIs(TYP_INT)); + intrinsic = NamedIntrinsic::NI_AVX2_ResetLowestSetBit; } LIR::Use use; @@ -6286,18 +6320,23 @@ GenTree* Lowering::TryLowerAndOpToExtractLowestSetBit(GenTreeOp* andNode) return nullptr; } - NamedIntrinsic intrinsic; - if (andNode->TypeIs(TYP_LONG) && m_compiler->compOpportunisticallyDependsOn(InstructionSet_AVX2_X64)) + if (!m_compiler->compOpportunisticallyDependsOn(InstructionSet_AVX2)) { - intrinsic = NamedIntrinsic::NI_AVX2_X64_ExtractLowestSetBit; + return nullptr; } - else if (m_compiler->compOpportunisticallyDependsOn(InstructionSet_AVX2)) + + NamedIntrinsic intrinsic; +#if defined(TARGET_AMD64) + if (andNode->TypeIs(TYP_LONG)) { - intrinsic = NamedIntrinsic::NI_AVX2_ExtractLowestSetBit; + intrinsic = NamedIntrinsic::NI_AVX2_X64_ExtractLowestSetBit; } else +#endif // TARGET_AMD64 { - return nullptr; + // On x86 longs are decomposed before lowering, so only the 32-bit form is reachable here. + assert(andNode->TypeIs(TYP_INT)); + intrinsic = NamedIntrinsic::NI_AVX2_ExtractLowestSetBit; } LIR::Use use; @@ -6325,6 +6364,280 @@ GenTree* Lowering::TryLowerAndOpToExtractLowestSetBit(GenTreeOp* andNode) return blsiNode; } +//---------------------------------------------------------------------------------------------- +// Lowering::TryLowerAndOpToZeroHighBits: Lowers a tree AND(X, ADD(LSH(1, Y), -1)) (or the +// equivalent AND(X, SUB(LSH(1, Y), 1))) to HWIntrinsic::ZeroHighBits (the BMI2 `bzhi` instruction), +// which zeroes the bits of X starting at bit position Y. +// +// Arguments: +// andNode - GT_AND node of integral type +// +// Return Value: +// Returns the replacement node if one is created else nullptr indicating no replacement +// +// Notes: +// Performs containment checks on the replacement node if one is created. +// +// `bzhi(x, y)` copies `x` and clears the bits at position `y` and higher. When `y >= width` +// it leaves `x` unchanged, which differs from the C# masked-shift result of `(1 << y) - 1`. +// ECMA-335 leaves the result of a shift by an amount >= the operand width unspecified and `y` +// here is a runtime value, so emitting `bzhi` is a legal implementation of the source pattern. +GenTree* Lowering::TryLowerAndOpToZeroHighBits(GenTreeOp* andNode) +{ + assert(andNode->OperIs(GT_AND) && varTypeIsIntegral(andNode)); + + if (!andNode->TypeIs(TYP_INT, TYP_LONG)) + { + return nullptr; + } + + // The mask `(1 << y) - 1` may be on either side of the AND. Global morph normally canonicalizes + // the subtraction to ADD(LSH(1, y), -1), but that only happens during global morph, so a SUB + // introduced afterwards (or by a later phase) can still reach here; recognize both forms. + GenTree* srcNode = nullptr; + GenTree* maskNode = nullptr; + if (andNode->gtGetOp2()->OperIs(GT_ADD, GT_SUB)) + { + maskNode = andNode->gtGetOp2(); + srcNode = andNode->gtGetOp1(); + } + else if (andNode->gtGetOp1()->OperIs(GT_ADD, GT_SUB)) + { + maskNode = andNode->gtGetOp1(); + srcNode = andNode->gtGetOp2(); + } + else + { + return nullptr; + } + + // maskNode must be ADD(LSH(1, y), -1) or the equivalent, un-canonicalized SUB(LSH(1, y), 1). + GenTree* lshNode = nullptr; + GenTree* constNode = nullptr; + if (maskNode->OperIs(GT_ADD) && maskNode->gtGetOp2()->IsIntegralConst(-1) && + maskNode->gtGetOp1()->OperIs(GT_LSH)) + { + lshNode = maskNode->gtGetOp1(); + constNode = maskNode->gtGetOp2(); + } + else if (maskNode->OperIs(GT_ADD) && maskNode->gtGetOp1()->IsIntegralConst(-1) && + maskNode->gtGetOp2()->OperIs(GT_LSH)) + { + lshNode = maskNode->gtGetOp2(); + constNode = maskNode->gtGetOp1(); + } + else if (maskNode->OperIs(GT_SUB) && maskNode->gtGetOp2()->IsIntegralConst(1) && + maskNode->gtGetOp1()->OperIs(GT_LSH)) + { + // Only SUB(LSH(1, y), 1) matches; SUB(1, LSH(1, y)) computes a different value. + lshNode = maskNode->gtGetOp1(); + constNode = maskNode->gtGetOp2(); + } + else + { + return nullptr; + } + + if (!lshNode->gtGetOp1()->IsIntegralConst(1)) + { + return nullptr; + } + GenTree* indexNode = lshNode->gtGetOp2(); + + // Subsequent nodes may rely on CPU flags set by these nodes in which case we cannot remove them + if (((andNode->gtFlags & GTF_SET_FLAGS) != 0) || ((maskNode->gtFlags & GTF_SET_FLAGS) != 0) || + ((lshNode->gtFlags & GTF_SET_FLAGS) != 0)) + { + return nullptr; + } + + if (!m_compiler->compOpportunisticallyDependsOn(InstructionSet_AVX2)) + { + return nullptr; + } + + NamedIntrinsic intrinsic; +#if defined(TARGET_AMD64) + if (andNode->TypeIs(TYP_LONG)) + { + intrinsic = NamedIntrinsic::NI_AVX2_X64_ZeroHighBits; + } + else +#endif // TARGET_AMD64 + { + // On x86 longs are decomposed before lowering, so only the 32-bit form is reachable here. + assert(andNode->TypeIs(TYP_INT)); + intrinsic = NamedIntrinsic::NI_AVX2_ZeroHighBits; + } + + LIR::Use use; + if (!BlockRange().TryGetUse(andNode, &use)) + { + return nullptr; + } + + // The backend expects op1 to be the index (encoded in VEX.vvvv) and op2 to be the value + // (which may be a memory operand), matching the import order for `ZeroHighBits`. + GenTreeHWIntrinsic* bzhiNode = + m_compiler->gtNewScalarHWIntrinsicNode(andNode->TypeGet(), indexNode, srcNode, intrinsic); + + JITDUMP("Lower: optimize AND(X, ADD(LSH(1, Y), -1))\n"); + DISPNODE(andNode); + JITDUMP("to:\n"); + DISPNODE(bzhiNode); + + BlockRange().InsertBefore(andNode, bzhiNode); + use.ReplaceWith(bzhiNode); + + BlockRange().Remove(andNode); + BlockRange().Remove(maskNode); + BlockRange().Remove(lshNode); + BlockRange().Remove(lshNode->gtGetOp1()); + BlockRange().Remove(constNode); + + ContainCheckHWIntrinsic(bzhiNode); + + return bzhiNode; +} + +//---------------------------------------------------------------------------------------------- +// Lowering::TryLowerBitwiseOpToBitOp: Lowers the single-bit manipulation idioms +// OR(X, LSH(1, Y)) -> GT_BIT_SET (`bts`, set bit Y of X) +// XOR(X, LSH(1, Y)) -> GT_BIT_INVERT (`btc`, complement bit Y of X) +// AND(X, NOT(LSH(1, Y))) -> GT_BIT_CLEAR (`btr`, reset bit Y of X) +// +// Arguments: +// binOp - GT_OR, GT_XOR, or GT_AND node of integral type +// +// Return Value: +// Returns the replacement node if one is created else nullptr indicating no replacement +// +// Notes: +// Only fires when the bit index Y is not a constant. For a constant index the shift folds to a +// constant mask that the existing `or`/`xor`/`and` with an immediate already handles optimally. +// +// The reg,reg form of `bts`/`btr`/`btc` masks the bit index modulo the operand width, which +// exactly matches the C# masked-shift semantics of `1 << Y`, so the transform is always valid +// (even when Y is out of range). The value operand is kept in a register because the +// memory-destination form of these instructions does not mask the index. +GenTree* Lowering::TryLowerBitwiseOpToBitOp(GenTreeOp* binOp) +{ + assert(binOp->OperIs(GT_OR, GT_XOR, GT_AND) && varTypeIsIntegral(binOp)); + + if (!binOp->TypeIs(TYP_INT, TYP_LONG)) + { + return nullptr; + } + + genTreeOps newOper; + switch (binOp->OperGet()) + { + case GT_OR: + newOper = GT_BIT_SET; + break; + case GT_XOR: + newOper = GT_BIT_INVERT; + break; + default: + assert(binOp->OperIs(GT_AND)); + newOper = GT_BIT_CLEAR; + break; + } + + // Locate the `1 << Y` sub-tree (wrapped in a NOT for the AND/`btr` case) which may be on either + // side of the commutative binary op. + GenTree* op1 = binOp->gtGetOp1(); + GenTree* op2 = binOp->gtGetOp2(); + GenTree* srcNode = nullptr; + GenTree* notNode = nullptr; + GenTree* lshNode = nullptr; + + auto matchShift = [&](GenTree* candidate) -> bool { + notNode = nullptr; + if (newOper == GT_BIT_CLEAR) + { + if (!candidate->OperIs(GT_NOT)) + { + return false; + } + notNode = candidate; + candidate = candidate->AsUnOp()->gtGetOp1(); + } + + if (!candidate->OperIs(GT_LSH)) + { + return false; + } + + lshNode = candidate; + return true; + }; + + if (matchShift(op2)) + { + srcNode = op1; + } + else if (matchShift(op1)) + { + srcNode = op2; + } + else + { + return nullptr; + } + + // The shifted value must be exactly `1` and match the width of the operation. + if (!lshNode->gtGetOp1()->IsIntegralConst(1) || !lshNode->TypeIs(binOp->TypeGet())) + { + return nullptr; + } + + // A constant index folds to a constant mask, which the plain form already handles optimally. + GenTree* indexNode = lshNode->gtGetOp2(); + if (indexNode->IsIntegralConst()) + { + return nullptr; + } + + // Subsequent nodes may rely on CPU flags set by these nodes in which case we cannot remove them. + if (((binOp->gtFlags & GTF_SET_FLAGS) != 0) || ((lshNode->gtFlags & GTF_SET_FLAGS) != 0) || + ((notNode != nullptr) && ((notNode->gtFlags & GTF_SET_FLAGS) != 0))) + { + return nullptr; + } + + LIR::Use use; + if (!BlockRange().TryGetUse(binOp, &use)) + { + return nullptr; + } + + GenTree* oneNode = lshNode->gtGetOp1(); + + // op1 is the value (read-modify-write destination), op2 is the bit index. + GenTree* bitOpNode = m_compiler->gtNewOperNode(newOper, binOp->TypeGet(), srcNode, indexNode); + + JITDUMP("Lower: optimize %s(X, %s)\n", GenTree::OpName(binOp->OperGet()), + (newOper == GT_BIT_CLEAR) ? "NOT(LSH(1, Y))" : "LSH(1, Y)"); + DISPNODE(binOp); + JITDUMP("to:\n"); + DISPNODE(bitOpNode); + + BlockRange().InsertBefore(binOp, bitOpNode); + use.ReplaceWith(bitOpNode); + + BlockRange().Remove(binOp); + if (notNode != nullptr) + { + BlockRange().Remove(notNode); + } + BlockRange().Remove(lshNode); + BlockRange().Remove(oneNode); + + // Both operands must remain in registers, so no containment is performed on the new node. + return bitOpNode; +} + //---------------------------------------------------------------------------------------------- // Lowering::TryLowerAndOpToAndNot: Lowers a tree AND(X, NOT(Y)) to HWIntrinsic::AndNot // @@ -6412,8 +6725,8 @@ GenTree* Lowering::TryLowerAndOpToAndNot(GenTreeOp* andNode) } //---------------------------------------------------------------------------------------------- -// Lowering::TryLowerXorOpToGetMaskUpToLowestSetBit: Lowers a tree XOR(X, ADD(X, -1)) to -// HWIntrinsic::GetMaskUpToLowestSetBit +// Lowering::TryLowerXorOpToGetMaskUpToLowestSetBit: Lowers a tree XOR(X, ADD(X, -1)) (or the +// equivalent XOR(X, SUB(X, 1))) to HWIntrinsic::GetMaskUpToLowestSetBit // // Arguments: // xorNode - GT_XOR node of integral type @@ -6434,13 +6747,26 @@ GenTree* Lowering::TryLowerXorOpToGetMaskUpToLowestSetBit(GenTreeOp* xorNode) } GenTree* op2 = xorNode->gtGetOp2(); - if (!op2->OperIs(GT_ADD)) + + // op2 must be ADD(X, -1), or the equivalent, un-canonicalized SUB(X, 1). Global morph normally + // rewrites the subtraction into the addition form, but that only happens during global morph so + // a SUB introduced afterwards can still reach here. + ssize_t expectedConst; + if (op2->OperIs(GT_ADD)) + { + expectedConst = -1; + } + else if (op2->OperIs(GT_SUB)) + { + expectedConst = 1; + } + else { return nullptr; } GenTree* addOp2 = op2->gtGetOp2(); - if (!addOp2->IsIntegralConst(-1)) + if (!addOp2->IsIntegralConst(expectedConst)) { return nullptr; } @@ -6458,18 +6784,23 @@ GenTree* Lowering::TryLowerXorOpToGetMaskUpToLowestSetBit(GenTreeOp* xorNode) return nullptr; } - NamedIntrinsic intrinsic; - if (xorNode->TypeIs(TYP_LONG) && m_compiler->compOpportunisticallyDependsOn(InstructionSet_AVX2_X64)) + if (!m_compiler->compOpportunisticallyDependsOn(InstructionSet_AVX2)) { - intrinsic = NamedIntrinsic::NI_AVX2_X64_GetMaskUpToLowestSetBit; + return nullptr; } - else if (m_compiler->compOpportunisticallyDependsOn(InstructionSet_AVX2)) + + NamedIntrinsic intrinsic; +#if defined(TARGET_AMD64) + if (xorNode->TypeIs(TYP_LONG)) { - intrinsic = NamedIntrinsic::NI_AVX2_GetMaskUpToLowestSetBit; + intrinsic = NamedIntrinsic::NI_AVX2_X64_GetMaskUpToLowestSetBit; } else +#endif // TARGET_AMD64 { - return nullptr; + // On x86 longs are decomposed before lowering, so only the 32-bit form is reachable here. + assert(xorNode->TypeIs(TYP_INT)); + intrinsic = NamedIntrinsic::NI_AVX2_GetMaskUpToLowestSetBit; } LIR::Use use; diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp index 58a9e2cbef9cc6..7eec5a85b6093f 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -315,6 +315,9 @@ int LinearScan::BuildNode(GenTree* tree) case GT_AND: case GT_OR: case GT_XOR: + case GT_BIT_SET: + case GT_BIT_CLEAR: + case GT_BIT_INVERT: srcCount = BuildBinaryUses(tree->AsOp()); assert(dstCount == 1); BuildDef(tree); diff --git a/src/tests/JIT/opt/InstructionCombining/Bzhi.cs b/src/tests/JIT/opt/InstructionCombining/Bzhi.cs new file mode 100644 index 00000000000000..cd9c122505323e --- /dev/null +++ b/src/tests/JIT/opt/InstructionCombining/Bzhi.cs @@ -0,0 +1,46 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.CompilerServices; +using Xunit; + +// Correctness coverage for the xarch `bzhi` (ZeroHighBits) lowering. +// +// The transform is opportunistic on AVX2, so this test intentionally avoids disasm checks (which +// would be flaky on hardware without AVX2) and instead validates that the optimized expression +// produces the same results as an independent oracle. The oracle is parameterized on runtime +// values so it does not itself fold into `bzhi`. +public static class Bzhi +{ + // Oracle for `x & ((1 << y) - 1)` restricted to the well-defined in-range domain [0, width). + [MethodImpl(MethodImplOptions.NoInlining)] + static int ZeroHighRef(int x, int y) => x & (int)(((1u << y) - 1)); + + [MethodImpl(MethodImplOptions.NoInlining)] + static long ZeroHighRef(long x, int y) => x & (long)(((1ul << y) - 1)); + + // bzhi candidates (variable index). + [MethodImpl(MethodImplOptions.NoInlining)] static int Bzhi_I(int x, int y) => x & ((1 << y) - 1); + [MethodImpl(MethodImplOptions.NoInlining)] static long Bzhi_L(long x, int y) => x & ((1L << y) - 1); + + [Fact] + public static void Test() + { + var rng = new Random(12345); + + for (int i = 0; i < 5000; i++) + { + uint xu = (uint)rng.Next() ^ ((uint)rng.Next() << 1); + int xi = (int)xu; + ulong xul = ((ulong)xu << 32) | (uint)rng.Next(); + long xl = (long)xul; + + // Restrict bzhi validation to the well-defined [0, width) index range. + int yi = rng.Next(0, 32); + int yl = rng.Next(0, 64); + Assert.Equal(ZeroHighRef(xi, yi), Bzhi_I(xi, yi)); + Assert.Equal(ZeroHighRef(xl, yl), Bzhi_L(xl, yl)); + } + } +} diff --git a/src/tests/JIT/opt/InstructionCombining/Bzhi.csproj b/src/tests/JIT/opt/InstructionCombining/Bzhi.csproj new file mode 100644 index 00000000000000..54dfc20c185b6d --- /dev/null +++ b/src/tests/JIT/opt/InstructionCombining/Bzhi.csproj @@ -0,0 +1,15 @@ + + + + true + + + None + True + + + + + + + diff --git a/src/tests/JIT/opt/InstructionCombining/SingleBit.cs b/src/tests/JIT/opt/InstructionCombining/SingleBit.cs index 4f0c267720850f..78a73197f6e110 100644 --- a/src/tests/JIT/opt/InstructionCombining/SingleBit.cs +++ b/src/tests/JIT/opt/InstructionCombining/SingleBit.cs @@ -8,13 +8,34 @@ public static class SingleBit { [MethodImpl(MethodImplOptions.NoInlining)] - static int Set(int a, int b) => a | (1 << b); + static int Set(int a, int b) + { + // X64: bts {{[a-z]+}}, {{[a-z]+}} + return a | (1 << b); + } [MethodImpl(MethodImplOptions.NoInlining)] - static int SetSwap(int a, int b) => (1 << b) | a ; + static int SetSwap(int a, int b) + { + // X64: bts {{[a-z]+}}, {{[a-z]+}} + return (1 << b) | a; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static long SetLong(long a, int b) + { + // X64: bts {{[a-z]+}}, {{[a-z]+}} + return a | (1L << b); + } [MethodImpl(MethodImplOptions.NoInlining)] - static int Set10(int a) => a | (1 << 10); + static int Set10(int a) + { + // A constant bit index folds to a plain 'or' with an immediate; it must not use 'bts'. + // X64-NOT: bts + // X64: or {{[a-z]+}}, 0x400 + return a | (1 << 10); + } [MethodImpl(MethodImplOptions.NoInlining)] static int Set11(int a) => a | (1 << 11); @@ -27,10 +48,25 @@ public static class SingleBit [MethodImpl(MethodImplOptions.NoInlining)] - static int Clear(int a, int b) => a & ~(1 << b); + static int Clear(int a, int b) + { + // X64: btr {{[a-z]+}}, {{[a-z]+}} + return a & ~(1 << b); + } [MethodImpl(MethodImplOptions.NoInlining)] - static int ClearSwap(int a, int b) => ~(1 << b) & a; + static int ClearSwap(int a, int b) + { + // X64: btr {{[a-z]+}}, {{[a-z]+}} + return ~(1 << b) & a; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static long ClearLong(long a, int b) + { + // X64: btr {{[a-z]+}}, {{[a-z]+}} + return a & ~(1L << b); + } [MethodImpl(MethodImplOptions.NoInlining)] static int Clear10(int a) => a & ~(1 << 10); @@ -49,10 +85,25 @@ public static class SingleBit [MethodImpl(MethodImplOptions.NoInlining)] - static int Invert(int a, int b) => a ^ (1 << b); + static int Invert(int a, int b) + { + // X64: btc {{[a-z]+}}, {{[a-z]+}} + return a ^ (1 << b); + } [MethodImpl(MethodImplOptions.NoInlining)] - static int InvertSwap(int a, int b) => (1 << b) ^ a; + static int InvertSwap(int a, int b) + { + // X64: btc {{[a-z]+}}, {{[a-z]+}} + return (1 << b) ^ a; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static long InvertLong(long a, int b) + { + // X64: btc {{[a-z]+}}, {{[a-z]+}} + return a ^ (1L << b); + } [MethodImpl(MethodImplOptions.NoInlining)] static int Invert10(int a) => a ^ (1 << 10); @@ -104,5 +155,16 @@ public static void Test() Assert.Equal(int.MinValue, Invert31(0)); Assert.Equal(-1, InvertNegatedBit(0, 0 + 32)); Assert.Equal(-4, InvertNegatedBit(0, 1 + 32)); + + // Long variants (exercise the 64-bit bts/btr/btc forms). The reg,reg encoding masks the + // bit index modulo 64, matching the C# masked-shift semantics even for out-of-range indices. + Assert.Equal(0x1_00000000L, SetLong(0, 32)); + Assert.Equal(0x1_00000000L, SetLong(0, 32 + 64)); + Assert.Equal(unchecked((long)0x8000000000000000UL), SetLong(0, 63)); + Assert.Equal(0x12345078L, ClearLong(0x1_12345078L, 32)); + Assert.Equal(0L, ClearLong(unchecked((long)0x8000000000000000UL), 63)); + Assert.Equal(0x1_00000000L, InvertLong(0, 32)); + Assert.Equal(0L, InvertLong(0x1_00000000L, 32)); + Assert.Equal(unchecked((long)0x8000000000000000UL), InvertLong(0, 63)); } } \ No newline at end of file diff --git a/src/tests/JIT/opt/InstructionCombining/SingleBit.csproj b/src/tests/JIT/opt/InstructionCombining/SingleBit.csproj index ebb108bc571dfe..1abbe59d35840d 100644 --- a/src/tests/JIT/opt/InstructionCombining/SingleBit.csproj +++ b/src/tests/JIT/opt/InstructionCombining/SingleBit.csproj @@ -8,7 +8,9 @@ True - + + true + From 42232f890424aa9324af5f1bac2e5e4c197cd0e1 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 9 Jul 2026 20:36:32 -0700 Subject: [PATCH 2/8] Recognize (x >> y) & 1 as a bit test and emit bt on xarch Extend tryReduceSingleBitTestOps to also match AND(RSH|RSZ(x, y), 1) so that a single-bit test written as ((x >> y) & 1) feeding a branch lowers to bt, the same as the already-handled x & (1 << y) shape. Only bit 0 of the shifted value is kept so the shift kind is irrelevant, and bt masks the bit index modulo the operand size, matching the C# masked-shift semantics even for an out-of-range y. Restricted to a variable index since a constant index keeps the shift and bt has no immediate form here (the existing constant-mask test is already optimal). The value operand's containment is cleared because ContainCheckCompare is skipped on success and the reg,reg bt form requires it in a register. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/lower.cpp | 28 +++++++ .../JIT/opt/InstructionCombining/SingleBit.cs | 77 +++++++++++++++++++ 2 files changed, 105 insertions(+) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index ad2728678ca593..b83d101449e83c 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -4235,6 +4235,34 @@ GenTree* Lowering::OptimizeConstCompare(GenTree* cmp) test->gtOp2 = bitOp->gtGetOp2(); return true; } + +#ifdef TARGET_XARCH + // Also recognize the arithmetic form `(x >> y) & 1`, i.e. AND(RSH|RSZ(x, y), 1), which + // tests bit `y` of `x` just like `x & (1 << y)`. Only bit 0 of the shifted value is kept so + // the shift kind is irrelevant, and `bt` masks the bit index modulo the operand size, which + // matches the C# masked-shift semantics even for an out-of-range `y`. Restricted to a + // variable index because a constant index keeps the shift, and `bt` has no immediate form + // here (a constant mask `test` is already optimal). + GenTree* shiftOp = test->gtOp1; + GenTree* oneOp = test->gtOp2; + if (!oneOp->IsIntegralConst(1)) + std::swap(shiftOp, oneOp); + + if (oneOp->IsIntegralConst(1) && shiftOp->OperIs(GT_RSH, GT_RSZ) && varTypeIsIntOrI(shiftOp) && + !shiftOp->gtGetOp2()->IsIntegralConst()) + { + BlockRange().Remove(oneOp); + BlockRange().Remove(shiftOp); + test->gtOp1 = shiftOp->gtGetOp1(); + test->gtOp2 = shiftOp->gtGetOp2(); + + // ContainCheckCompare is skipped when this transform succeeds, so clear any containment + // the value operand picked up from the removed shift (e.g. a `shrx` memory source) -- + // the reg,reg `bt` form requires it in a register. + test->gtOp1->ClearContained(); + return true; + } +#endif // TARGET_XARCH return false; }; diff --git a/src/tests/JIT/opt/InstructionCombining/SingleBit.cs b/src/tests/JIT/opt/InstructionCombining/SingleBit.cs index 78a73197f6e110..e98338006b5978 100644 --- a/src/tests/JIT/opt/InstructionCombining/SingleBit.cs +++ b/src/tests/JIT/opt/InstructionCombining/SingleBit.cs @@ -117,6 +117,68 @@ static long InvertLong(long a, int b) [MethodImpl(MethodImplOptions.NoInlining)] static int InvertNegatedBit(int a, int b) => ~(1 << a) ^ (1 << b); + // Bit-test recognition: testing a single bit to feed a branch should become 'bt'. Both the + // '(x >> y) & 1' and 'x & (1 << y)' shapes select bit 'y', and 'bt' masks the index modulo the + // operand size, matching the C# masked-shift semantics even for an out-of-range 'y'. + + [MethodImpl(MethodImplOptions.NoInlining)] + static int TestBitShr(int a, int b) + { + // X64: bt {{[a-z]+}}, {{[a-z]+}} + if (((a >> b) & 1) != 0) + { + return 100; + } + return 200; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int TestBitShrEq(int a, int b) + { + // X64: bt {{[a-z]+}}, {{[a-z]+}} + if (((a >> b) & 1) == 0) + { + return 100; + } + return 200; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int TestBitShrLong(long a, int b) + { + // X64: bt {{[a-z]+}}, {{[a-z]+}} + if (((a >> b) & 1) != 0) + { + return 100; + } + return 200; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int TestBitMask(int a, int b) + { + // X64: bt {{[a-z]+}}, {{[a-z]+}} + if ((a & (1 << b)) != 0) + { + return 100; + } + return 200; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int TestBitShrConst(int a) + { + // A constant bit index keeps the shift folded into a 'test' with an immediate; 'bt' has no + // immediate form here so it must not be used. + // X64-NOT: bt + // X64: test {{[a-z]+}}, {{(32|0x20)}} + if (((a >> 5) & 1) != 0) + { + return 100; + } + return 200; + } + [Fact] public static void Test() { @@ -166,5 +228,20 @@ public static void Test() Assert.Equal(0x1_00000000L, InvertLong(0, 32)); Assert.Equal(0L, InvertLong(0x1_00000000L, 32)); Assert.Equal(unchecked((long)0x8000000000000000UL), InvertLong(0, 63)); + + // Bit-test recognition. Exercise a set and a clear bit, plus an out-of-range (masked) index. + Assert.Equal(100, TestBitShr(0b1000, 3)); + Assert.Equal(200, TestBitShr(0b1000, 2)); + Assert.Equal(100, TestBitShr(0b1000, 3 + 32)); + Assert.Equal(200, TestBitShrEq(0b1000, 3)); + Assert.Equal(100, TestBitShrEq(0b1000, 2)); + Assert.Equal(100, TestBitShrLong(0x1_00000000L, 32)); + Assert.Equal(200, TestBitShrLong(0x1_00000000L, 33)); + Assert.Equal(100, TestBitShrLong(0x1_00000000L, 32 + 64)); + Assert.Equal(100, TestBitMask(0b1000, 3)); + Assert.Equal(200, TestBitMask(0b1000, 2)); + Assert.Equal(100, TestBitMask(0b1000, 3 + 32)); + Assert.Equal(100, TestBitShrConst(0b100000)); + Assert.Equal(200, TestBitShrConst(0b010000)); } } \ No newline at end of file From 2235f7fb0475f9382c334191eaf77fc995e5eec9 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Fri, 10 Jul 2026 05:05:35 -0700 Subject: [PATCH 3/8] Apply formatting patch --- src/coreclr/jit/lowerxarch.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/coreclr/jit/lowerxarch.cpp b/src/coreclr/jit/lowerxarch.cpp index f16312fcf73a05..f147a8817e6fbe 100644 --- a/src/coreclr/jit/lowerxarch.cpp +++ b/src/coreclr/jit/lowerxarch.cpp @@ -6414,8 +6414,7 @@ GenTree* Lowering::TryLowerAndOpToZeroHighBits(GenTreeOp* andNode) // maskNode must be ADD(LSH(1, y), -1) or the equivalent, un-canonicalized SUB(LSH(1, y), 1). GenTree* lshNode = nullptr; GenTree* constNode = nullptr; - if (maskNode->OperIs(GT_ADD) && maskNode->gtGetOp2()->IsIntegralConst(-1) && - maskNode->gtGetOp1()->OperIs(GT_LSH)) + if (maskNode->OperIs(GT_ADD) && maskNode->gtGetOp2()->IsIntegralConst(-1) && maskNode->gtGetOp1()->OperIs(GT_LSH)) { lshNode = maskNode->gtGetOp1(); constNode = maskNode->gtGetOp2(); From c2df4f1f7f959866696f1cfbb2cb616b879c535b Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Fri, 10 Jul 2026 05:59:07 -0700 Subject: [PATCH 4/8] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/coreclr/jit/lowerxarch.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/lowerxarch.cpp b/src/coreclr/jit/lowerxarch.cpp index f147a8817e6fbe..fcc3e8b69444e3 100644 --- a/src/coreclr/jit/lowerxarch.cpp +++ b/src/coreclr/jit/lowerxarch.cpp @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. /*XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX From 353fbec05db0e7e192d4dd2b8f3deeee0296a4d1 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Fri, 10 Jul 2026 07:18:26 -0700 Subject: [PATCH 5/8] Mask the bzhi index to preserve C# masked-shift semantics C# defines 1 << y as masking the shift count modulo the operand width, and this JIT already models shl/shr as masked (LowerShift drops redundant & 31/ & 63). zhi instead leaves the source unchanged when the index is >= width, so lowering AND(x, (1 << y) - 1) to zhi(x, y) miscompiled the source pattern for out-of-range y (e.g. Bzhi_I(x, 32) returned x instead of 0). Mask the index to the operand width before zhi so it reproduces the masked-shift result exactly; the extra nd reg, imm is contained and the transform stays a net size/PerfScore win in SPMI (-3,425 bytes overall, all collections green). Also address related review feedback: rewrite the header comment to document the masked-index invariant, move TryLowerBitwiseOpToBitOp out of the FEATURE_HW_INTRINSICS region (bts/btr/btc are not hardware intrinsics), add a defensive assert in genCodeForBitOp documenting the LSRA delayFree invariant on the bit index, and expand the Bzhi test to cover indices past the operand width via an oracle that routes through an opaque masked shift. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/codegenxarch.cpp | 5 +++ src/coreclr/jit/lower.h | 2 +- src/coreclr/jit/lowerxarch.cpp | 30 +++++++++++++--- .../JIT/opt/InstructionCombining/Bzhi.cs | 34 +++++++++++++------ 4 files changed, 55 insertions(+), 16 deletions(-) diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index 74de7a53977a07..e5411b71cd8f22 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -1156,6 +1156,11 @@ void CodeGen::genCodeForBitOp(GenTreeOp* treeNode) instruction ins = treeNode->OperIs(GT_BIT_SET) ? INS_bts : treeNode->OperIs(GT_BIT_CLEAR) ? INS_btr : INS_btc; + // LSRA marks op2 (the bit index) as delayFree for these read-modify-write nodes, so the target + // register can never alias the index. That guarantees the `mov` below (which loads the value into + // the destination) does not clobber the index before `bts`/`btr`/`btc` reads it. + assert(targetReg != op2->GetRegNum()); + // These are read-modify-write: the destination register also supplies the value operand. inst_Mov(targetType, targetReg, op1->GetRegNum(), /* canSkip */ true); diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index f70806c562b4a1..9e0c5263eec19b 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -454,6 +454,7 @@ class Lowering final : public Phase #ifdef TARGET_XARCH GenTree* TryLowerMulWithConstant(GenTreeOp* node); + GenTree* TryLowerBitwiseOpToBitOp(GenTreeOp* binOp); #endif // TARGET_XARCH #ifdef TARGET_WASM @@ -511,7 +512,6 @@ class Lowering final : public Phase GenTree* TryLowerAndOpToAndNot(GenTreeOp* andNode); GenTree* TryLowerAndOpToZeroHighBits(GenTreeOp* andNode); GenTree* TryLowerXorOpToGetMaskUpToLowestSetBit(GenTreeOp* xorNode); - GenTree* TryLowerBitwiseOpToBitOp(GenTreeOp* binOp); void LowerBswapOp(GenTreeOp* node); GenTree* LowerHWIntrinsicDotInnerMulSum(GenTreeHWIntrinsic* node); #elif defined(TARGET_ARM64) diff --git a/src/coreclr/jit/lowerxarch.cpp b/src/coreclr/jit/lowerxarch.cpp index fcc3e8b69444e3..0a5be8282d480d 100644 --- a/src/coreclr/jit/lowerxarch.cpp +++ b/src/coreclr/jit/lowerxarch.cpp @@ -6378,10 +6378,12 @@ GenTree* Lowering::TryLowerAndOpToExtractLowestSetBit(GenTreeOp* andNode) // Notes: // Performs containment checks on the replacement node if one is created. // -// `bzhi(x, y)` copies `x` and clears the bits at position `y` and higher. When `y >= width` -// it leaves `x` unchanged, which differs from the C# masked-shift result of `(1 << y) - 1`. -// ECMA-335 leaves the result of a shift by an amount >= the operand width unspecified and `y` -// here is a runtime value, so emitting `bzhi` is a legal implementation of the source pattern. +// The bit index is masked to the operand width (`y & 31` for `int`, `y & 63` for `long`) before +// being handed to `bzhi`. C#'s `<<` masks the shift count modulo the operand width, so the mask +// `(1 << y) - 1` is well-defined for any `y` and this JIT already models `shl`/`shr` as masked +// (see `LowerShift`, which drops redundant `& 31`/`& 63`). `bzhi`, in contrast, leaves the source +// unchanged when the index is `>= width`, so without masking the index it would diverge from the +// source pattern for those inputs. Masking the index makes `bzhi` reproduce the result exactly. GenTree* Lowering::TryLowerAndOpToZeroHighBits(GenTreeOp* andNode) { assert(andNode->OperIs(GT_AND) && varTypeIsIntegral(andNode)); @@ -6477,14 +6479,24 @@ GenTree* Lowering::TryLowerAndOpToZeroHighBits(GenTreeOp* andNode) // The backend expects op1 to be the index (encoded in VEX.vvvv) and op2 to be the value // (which may be a memory operand), matching the import order for `ZeroHighBits`. + // + // Mask the index modulo the operand width so `bzhi` reproduces the C# masked-shift semantics of + // `1 << Y` even when `Y >= width` (where `bzhi` would otherwise leave the source unchanged). The + // index is never a constant here (a constant `1 << Y` folds to a constant mask during morph), so + // the mask is always applied to a variable and cannot be folded away. + GenTree* maskCns = m_compiler->gtNewIconNode(andNode->TypeIs(TYP_LONG) ? 63 : 31, genActualType(indexNode)); + GenTree* indexMask = m_compiler->gtNewOperNode(GT_AND, genActualType(indexNode), indexNode, maskCns); + GenTreeHWIntrinsic* bzhiNode = - m_compiler->gtNewScalarHWIntrinsicNode(andNode->TypeGet(), indexNode, srcNode, intrinsic); + m_compiler->gtNewScalarHWIntrinsicNode(andNode->TypeGet(), indexMask, srcNode, intrinsic); JITDUMP("Lower: optimize AND(X, ADD(LSH(1, Y), -1))\n"); DISPNODE(andNode); JITDUMP("to:\n"); DISPNODE(bzhiNode); + BlockRange().InsertBefore(andNode, maskCns); + BlockRange().InsertBefore(andNode, indexMask); BlockRange().InsertBefore(andNode, bzhiNode); use.ReplaceWith(bzhiNode); @@ -6494,11 +6506,17 @@ GenTree* Lowering::TryLowerAndOpToZeroHighBits(GenTreeOp* andNode) BlockRange().Remove(lshNode->gtGetOp1()); BlockRange().Remove(constNode); + ContainCheckBinary(indexMask->AsOp()); ContainCheckHWIntrinsic(bzhiNode); return bzhiNode; } +#endif // FEATURE_HW_INTRINSICS + +// bts/btr/btc are not hardware intrinsics, so TryLowerBitwiseOpToBitOp lives outside the +// FEATURE_HW_INTRINSICS region even though its neighbors above/below use HWIntrinsic nodes. + //---------------------------------------------------------------------------------------------- // Lowering::TryLowerBitwiseOpToBitOp: Lowers the single-bit manipulation idioms // OR(X, LSH(1, Y)) -> GT_BIT_SET (`bts`, set bit Y of X) @@ -6637,6 +6655,8 @@ GenTree* Lowering::TryLowerBitwiseOpToBitOp(GenTreeOp* binOp) return bitOpNode; } +#ifdef FEATURE_HW_INTRINSICS + //---------------------------------------------------------------------------------------------- // Lowering::TryLowerAndOpToAndNot: Lowers a tree AND(X, NOT(Y)) to HWIntrinsic::AndNot // diff --git a/src/tests/JIT/opt/InstructionCombining/Bzhi.cs b/src/tests/JIT/opt/InstructionCombining/Bzhi.cs index cd9c122505323e..66e366574815ab 100644 --- a/src/tests/JIT/opt/InstructionCombining/Bzhi.cs +++ b/src/tests/JIT/opt/InstructionCombining/Bzhi.cs @@ -9,16 +9,21 @@ // // The transform is opportunistic on AVX2, so this test intentionally avoids disasm checks (which // would be flaky on hardware without AVX2) and instead validates that the optimized expression -// produces the same results as an independent oracle. The oracle is parameterized on runtime -// values so it does not itself fold into `bzhi`. +// produces the same results as an independent oracle. +// +// C#'s `<<` masks the shift count modulo the operand width, so `x & ((1 << y) - 1)` is well-defined +// for *any* y (e.g. `1 << 32 == 1 << 0`). The lowering must reproduce that for out-of-range y too, +// so the index range below deliberately spans well past the operand width. The oracle routes the +// shift through a NoInlining `1 << y` helper: that emits a bare (masked) `shl` which is never itself +// recognized as `bzhi`, so it is an independent reference rather than a copy of the candidate. public static class Bzhi { - // Oracle for `x & ((1 << y) - 1)` restricted to the well-defined in-range domain [0, width). - [MethodImpl(MethodImplOptions.NoInlining)] - static int ZeroHighRef(int x, int y) => x & (int)(((1u << y) - 1)); + [MethodImpl(MethodImplOptions.NoInlining)] static int ShlOne(int y) => 1 << y; + [MethodImpl(MethodImplOptions.NoInlining)] static long ShlOneL(int y) => 1L << y; - [MethodImpl(MethodImplOptions.NoInlining)] - static long ZeroHighRef(long x, int y) => x & (long)(((1ul << y) - 1)); + // Oracle for `x & ((1 << y) - 1)` using masked-shift (C#-defined) semantics. + static int ZeroHighRef(int x, int y) => x & (ShlOne(y) - 1); + static long ZeroHighRef(long x, int y) => x & (ShlOneL(y) - 1); // bzhi candidates (variable index). [MethodImpl(MethodImplOptions.NoInlining)] static int Bzhi_I(int x, int y) => x & ((1 << y) - 1); @@ -29,6 +34,9 @@ public static void Test() { var rng = new Random(12345); + // Boundary indices around and beyond both operand widths (32 and 64). + int[] boundaries = { 0, 1, 15, 30, 31, 32, 33, 63, 64, 65, 95, 96, 127, 128, 200, 255 }; + for (int i = 0; i < 5000; i++) { uint xu = (uint)rng.Next() ^ ((uint)rng.Next() << 1); @@ -36,9 +44,15 @@ public static void Test() ulong xul = ((ulong)xu << 32) | (uint)rng.Next(); long xl = (long)xul; - // Restrict bzhi validation to the well-defined [0, width) index range. - int yi = rng.Next(0, 32); - int yl = rng.Next(0, 64); + foreach (int y in boundaries) + { + Assert.Equal(ZeroHighRef(xi, y), Bzhi_I(xi, y)); + Assert.Equal(ZeroHighRef(xl, y), Bzhi_L(xl, y)); + } + + // Random indices spanning past the operand width to exercise masked-shift semantics. + int yi = rng.Next(0, 256); + int yl = rng.Next(0, 256); Assert.Equal(ZeroHighRef(xi, yi), Bzhi_I(xi, yi)); Assert.Equal(ZeroHighRef(xl, yl), Bzhi_L(xl, yl)); } From 31d730947dda821f857053a774c868e88aa24461 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Fri, 10 Jul 2026 07:23:54 -0700 Subject: [PATCH 6/8] Widen SingleBit FileCheck register patterns to match r8-r15 The disasm checks used `{{[a-z]+}}` for register operands, which only matches registers made purely of letters (eax/ecx/rax). When register allocation places an operand in r8-r15 (e.g. `bts r8d, r10d`) the pattern fails to match and the check spuriously fails; it passes today only when RA happens to pick a lettered register. Widen the operand patterns to `{{[a-z0-9]+}}`. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../JIT/opt/InstructionCombining/SingleBit.cs | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/tests/JIT/opt/InstructionCombining/SingleBit.cs b/src/tests/JIT/opt/InstructionCombining/SingleBit.cs index e98338006b5978..f52db53cbba028 100644 --- a/src/tests/JIT/opt/InstructionCombining/SingleBit.cs +++ b/src/tests/JIT/opt/InstructionCombining/SingleBit.cs @@ -10,21 +10,21 @@ public static class SingleBit [MethodImpl(MethodImplOptions.NoInlining)] static int Set(int a, int b) { - // X64: bts {{[a-z]+}}, {{[a-z]+}} + // X64: bts {{[a-z0-9]+}}, {{[a-z0-9]+}} return a | (1 << b); } [MethodImpl(MethodImplOptions.NoInlining)] static int SetSwap(int a, int b) { - // X64: bts {{[a-z]+}}, {{[a-z]+}} + // X64: bts {{[a-z0-9]+}}, {{[a-z0-9]+}} return (1 << b) | a; } [MethodImpl(MethodImplOptions.NoInlining)] static long SetLong(long a, int b) { - // X64: bts {{[a-z]+}}, {{[a-z]+}} + // X64: bts {{[a-z0-9]+}}, {{[a-z0-9]+}} return a | (1L << b); } @@ -33,7 +33,7 @@ static int Set10(int a) { // A constant bit index folds to a plain 'or' with an immediate; it must not use 'bts'. // X64-NOT: bts - // X64: or {{[a-z]+}}, 0x400 + // X64: or {{[a-z0-9]+}}, 0x400 return a | (1 << 10); } @@ -50,21 +50,21 @@ static int Set10(int a) [MethodImpl(MethodImplOptions.NoInlining)] static int Clear(int a, int b) { - // X64: btr {{[a-z]+}}, {{[a-z]+}} + // X64: btr {{[a-z0-9]+}}, {{[a-z0-9]+}} return a & ~(1 << b); } [MethodImpl(MethodImplOptions.NoInlining)] static int ClearSwap(int a, int b) { - // X64: btr {{[a-z]+}}, {{[a-z]+}} + // X64: btr {{[a-z0-9]+}}, {{[a-z0-9]+}} return ~(1 << b) & a; } [MethodImpl(MethodImplOptions.NoInlining)] static long ClearLong(long a, int b) { - // X64: btr {{[a-z]+}}, {{[a-z]+}} + // X64: btr {{[a-z0-9]+}}, {{[a-z0-9]+}} return a & ~(1L << b); } @@ -87,21 +87,21 @@ static long ClearLong(long a, int b) [MethodImpl(MethodImplOptions.NoInlining)] static int Invert(int a, int b) { - // X64: btc {{[a-z]+}}, {{[a-z]+}} + // X64: btc {{[a-z0-9]+}}, {{[a-z0-9]+}} return a ^ (1 << b); } [MethodImpl(MethodImplOptions.NoInlining)] static int InvertSwap(int a, int b) { - // X64: btc {{[a-z]+}}, {{[a-z]+}} + // X64: btc {{[a-z0-9]+}}, {{[a-z0-9]+}} return (1 << b) ^ a; } [MethodImpl(MethodImplOptions.NoInlining)] static long InvertLong(long a, int b) { - // X64: btc {{[a-z]+}}, {{[a-z]+}} + // X64: btc {{[a-z0-9]+}}, {{[a-z0-9]+}} return a ^ (1L << b); } @@ -124,7 +124,7 @@ static long InvertLong(long a, int b) [MethodImpl(MethodImplOptions.NoInlining)] static int TestBitShr(int a, int b) { - // X64: bt {{[a-z]+}}, {{[a-z]+}} + // X64: bt {{[a-z0-9]+}}, {{[a-z0-9]+}} if (((a >> b) & 1) != 0) { return 100; @@ -135,7 +135,7 @@ static int TestBitShr(int a, int b) [MethodImpl(MethodImplOptions.NoInlining)] static int TestBitShrEq(int a, int b) { - // X64: bt {{[a-z]+}}, {{[a-z]+}} + // X64: bt {{[a-z0-9]+}}, {{[a-z0-9]+}} if (((a >> b) & 1) == 0) { return 100; @@ -146,7 +146,7 @@ static int TestBitShrEq(int a, int b) [MethodImpl(MethodImplOptions.NoInlining)] static int TestBitShrLong(long a, int b) { - // X64: bt {{[a-z]+}}, {{[a-z]+}} + // X64: bt {{[a-z0-9]+}}, {{[a-z0-9]+}} if (((a >> b) & 1) != 0) { return 100; @@ -157,7 +157,7 @@ static int TestBitShrLong(long a, int b) [MethodImpl(MethodImplOptions.NoInlining)] static int TestBitMask(int a, int b) { - // X64: bt {{[a-z]+}}, {{[a-z]+}} + // X64: bt {{[a-z0-9]+}}, {{[a-z0-9]+}} if ((a & (1 << b)) != 0) { return 100; @@ -171,7 +171,7 @@ static int TestBitShrConst(int a) // A constant bit index keeps the shift folded into a 'test' with an immediate; 'bt' has no // immediate form here so it must not be used. // X64-NOT: bt - // X64: test {{[a-z]+}}, {{(32|0x20)}} + // X64: test {{[a-z0-9]+}}, {{(32|0x20)}} if (((a >> 5) & 1) != 0) { return 100; From 53bdcc22a56a93d51996193ed3f90e2ca8eea1f3 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Fri, 10 Jul 2026 07:55:49 -0700 Subject: [PATCH 7/8] Drop the over-strict bit-op target/index assert and match either immediate form The assert(targetReg != op2) added for the GT_BIT_* codegen is unsound: for x (1 << x) op1 and op2 share an interval, so AddDelayFreeUses skips the delayFree and LSRA may place the destination on op2's register. That is safe -- op1 and op2 hold the same value, so the mov writes op2's own value back -- but it can't be expressed as a cheap codegen assert. JitStressRegs=0x800 reaches the aliased form and computes the correct result. Also widen the Set10 FileCheck to accept 1024 or 0x400, matching the file's other immediate patterns. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/codegenxarch.cpp | 11 +++++++---- src/tests/JIT/opt/InstructionCombining/SingleBit.cs | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index e5411b71cd8f22..e58c95e201845e 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -1156,10 +1156,13 @@ void CodeGen::genCodeForBitOp(GenTreeOp* treeNode) instruction ins = treeNode->OperIs(GT_BIT_SET) ? INS_bts : treeNode->OperIs(GT_BIT_CLEAR) ? INS_btr : INS_btc; - // LSRA marks op2 (the bit index) as delayFree for these read-modify-write nodes, so the target - // register can never alias the index. That guarantees the `mov` below (which loads the value into - // the destination) does not clobber the index before `bts`/`btr`/`btc` reads it. - assert(targetReg != op2->GetRegNum()); + // These are read-modify-write: the `mov` below loads op1 (the value) into the destination and + // then `bts`/`btr`/`btc` reads the bit index from op2. LSRA marks op2 as delayFree except when + // op2 shares op1's interval and it's their last use -- i.e. `x (1 << x)`, where op1 and op2 + // are the same value (see AddDelayFreeUses). So the destination can only alias op2 when op1 and + // op2 hold the same value, in which case the `mov` writes that same value back into op2's + // register and nothing is clobbered before the bit-test reads it. When the operands are distinct + // values, delayFree guarantees the destination and op2 use different registers. // These are read-modify-write: the destination register also supplies the value operand. inst_Mov(targetType, targetReg, op1->GetRegNum(), /* canSkip */ true); diff --git a/src/tests/JIT/opt/InstructionCombining/SingleBit.cs b/src/tests/JIT/opt/InstructionCombining/SingleBit.cs index f52db53cbba028..48a0b8517109c2 100644 --- a/src/tests/JIT/opt/InstructionCombining/SingleBit.cs +++ b/src/tests/JIT/opt/InstructionCombining/SingleBit.cs @@ -33,7 +33,7 @@ static int Set10(int a) { // A constant bit index folds to a plain 'or' with an immediate; it must not use 'bts'. // X64-NOT: bts - // X64: or {{[a-z0-9]+}}, 0x400 + // X64: or {{[a-z0-9]+}}, {{(1024|0x400)}} return a | (1 << 10); } From 719be07b8c0847b500b6d0e140b82c0a481d193e Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Fri, 10 Jul 2026 08:54:30 -0700 Subject: [PATCH 8/8] Bail bzhi lowering on a constant bit index to honor the variable-index contract TryLowerAndOpToZeroHighBits documented that the shift count is never constant here (a constant 1 << Y folds to a constant mask during morph), but nothing enforced it. Like the un-canonicalized SUB form the function already guards against, a post-morph constant index could reach here and regress the optimal and reg, imm into mov reg, imm + bzhi, since bzhi has no immediate index form. Bail explicitly on a constant index. SPMI x64 confirms this is codegen-neutral: clean diff across all 10 collections (~2.6M contexts). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/lowerxarch.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/lowerxarch.cpp b/src/coreclr/jit/lowerxarch.cpp index 0a5be8282d480d..1aab07796fd360 100644 --- a/src/coreclr/jit/lowerxarch.cpp +++ b/src/coreclr/jit/lowerxarch.cpp @@ -6445,6 +6445,16 @@ GenTree* Lowering::TryLowerAndOpToZeroHighBits(GenTreeOp* andNode) } GenTree* indexNode = lshNode->gtGetOp2(); + // A constant shift count folds `(1 << Y) - 1` to a constant mask during morph, so a constant + // index should never reach here. Enforce that variable-index contract explicitly: `bzhi` takes + // its index in a register (there is no immediate form), so lowering a constant index would + // regress the optimal `and reg, imm` into `mov reg, imm` + `bzhi`. Bail and let the plain `and` + // stand -- this mirrors how the SUB form above guards against post-morph shapes. + if (indexNode->IsIntegralConst()) + { + return nullptr; + } + // Subsequent nodes may rely on CPU flags set by these nodes in which case we cannot remove them if (((andNode->gtFlags & GTF_SET_FLAGS) != 0) || ((maskNode->gtFlags & GTF_SET_FLAGS) != 0) || ((lshNode->gtFlags & GTF_SET_FLAGS) != 0)) @@ -6482,8 +6492,8 @@ GenTree* Lowering::TryLowerAndOpToZeroHighBits(GenTreeOp* andNode) // // Mask the index modulo the operand width so `bzhi` reproduces the C# masked-shift semantics of // `1 << Y` even when `Y >= width` (where `bzhi` would otherwise leave the source unchanged). The - // index is never a constant here (a constant `1 << Y` folds to a constant mask during morph), so - // the mask is always applied to a variable and cannot be folded away. + // index is guaranteed non-constant here (enforced by the bail above), so the mask is always + // applied to a variable and cannot be folded away. GenTree* maskCns = m_compiler->gtNewIconNode(andNode->TypeIs(TYP_LONG) ? 63 : 31, genActualType(indexNode)); GenTree* indexMask = m_compiler->gtNewOperNode(GT_AND, genActualType(indexNode), indexNode, maskCns);