Recognize bit-manipulation idioms and emit bzhi/bts/btc/btr/bt on xarch by tannergooding · Pull Request #130481 · dotnet/runtime

tannergooding · 2026-07-10T12:07:47Z

Lowers several single-bit and bit-field idioms to dedicated x86 instructions on xarch:

Idiom	Instruction
`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`)
`(x >> y) & 1` in `== 0` / `!= 0`	`bt` (`GT_BITTEST_EQ`/`GT_BITTEST_NE`)

Details

bzhi is recognized in LowerBinaryArithmetic and folded into the existing AVX2 scalar intrinsic, so containment and memory operands come 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.
bts/btr/btc 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.
bt recognizes (x >> y) & 1 feeding a == 0 / != 0 and emits the flag-producing bit test.

`bextr` deferred

#81514 also covers bextr, but constant-control bextr ((x >> c) & mask) is intentionally left out: it must materialize the control word and is multi-uop, so shr/sar + and in place is smaller and faster. SPMI confirmed it was a net regression (+7,671 bytes) and it only wins when the source is a live-after local (saving a preservation copy) â€” which needs last-use info not available during lowering. The fully-variable (x >> y) & ((1 << z) - 1) case already lowers optimally to shrx + bzhi, so nothing is lost.

A cost-aware follow-up prototyped the variable/mixed-control cases (the only ones not previously implemented) and measured them in isolation with SPMI asmdiffs over 2.6M contexts: +61 bytes, zero improvements, every diff a regression. Non-constant control does not avoid the materialization â€” it replaces the free and reg, imm mask with a multi-instruction packed-control build feeding a multi-uop bextr, and shrx/bzhi already provide the non-destructive form that was bextr's only structural edge. So bextr is left out in all quadrants.

Diffs

SPMI asmdiffs (x64 windows): overall -3,874 bytes, every collection improves in both size and PerfScore (coreclr_tests PerfScore -5.09%), no asserts, no replay failures. The only regressions are 6 methods at +1/+2 bytes each (memory-RMW bts where the memory destination can't be contained without breaking the bit-index masking) â€” negligible against the wins.

Fixes #81514
Fixes #81512

Note

This PR description was generated with the assistance of GitHub Copilot.

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 dotnet#81514 Fixes dotnet#81512 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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>

Copilot

Pull request overview

This PR extends xarch JIT lowering/codegen to recognize common single-bit and bit-field idioms and emit dedicated x86 instructions (bts/btr/btc, bt, and bzhi via the existing ZeroHighBits intrinsic). It also adds/updates JIT tests to validate the new instruction selection.

Changes:

Add lowering for single-bit set/clear/invert idioms to new GT_BIT_SET/GT_BIT_CLEAR/GT_BIT_INVERT nodes and emit bts/btr/btc in xarch codegen.
Extend compare/test optimization to recognize the ((x >> y) & 1) shape and lower it to GT_BITTEST_EQ/NE (xarch bt).
Add/adjust instruction-combining tests, including disasm checks for the new bit ops and a new bzhi correctness test.

Show a summary per file

File	Description
src/tests/JIT/opt/InstructionCombining/SingleBit.csproj	Enables disasm checks for `SingleBit.cs`.
src/tests/JIT/opt/InstructionCombining/SingleBit.cs	Adds disasm-validated coverage for `bts`/`btr`/`btc` and `bt`, plus 64-bit variants.
src/tests/JIT/opt/InstructionCombining/Bzhi.csproj	Adds a new instruction-combining test project for `bzhi` coverage.
src/tests/JIT/opt/InstructionCombining/Bzhi.cs	Adds randomized correctness checks intended to cover `x & ((1 << y) - 1)` lowering.
src/coreclr/jit/lsraxarch.cpp	Teaches LSRA to treat `GT_BIT_*` nodes as binary ops for use/def modeling.
src/coreclr/jit/lowerxarch.cpp	Adds new pattern-based lowering for `bzhi` and `GT_BIT_*` nodes; adjusts some existing bit-idiom lowering to accept `SUB` forms.
src/coreclr/jit/lower.h	Declares the new xarch lowering helpers.
src/coreclr/jit/lower.cpp	Extends const-compare optimization to recognize `((x >> y) & 1)` for bit-test lowering.
src/coreclr/jit/instrsxarch.h	Adds instruction definitions for `bts`/`btr`/`btc`.
src/coreclr/jit/emitxarch.cpp	Adjusts instruction display for BT-family operand reversal for `bts`/`btr`/`btc`.
src/coreclr/jit/codegenxarch.cpp	Implements codegen for `GT_BIT_SET/CLEAR/INVERT` using `bts/btr/btc`.
src/coreclr/jit/codegen.h	Declares `genCodeForBitOp`.
src/coreclr/jit/gtlist.h	Enables `GT_BIT_*` nodes for xarch (shared with riscv64).

Copilot's findings

Files reviewed: 13/13 changed files
Comments generated: 7

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Copilot's findings

Files reviewed: 13/13 changed files
Comments generated: 10

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>

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>

Copilot

Copilot's findings

Files reviewed: 13/13 changed files
Comments generated: 3

Copilot

Copilot's findings

Files reviewed: 13/13 changed files
Comments generated: 0 new

…diate form The assert(targetReg != op2) added for the GT_BIT_* codegen is unsound: for x <op> (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>

Copilot

Copilot's findings

Files reviewed: 13/13 changed files
Comments generated: 1

…x 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>

Copilot

Copilot's findings

Files reviewed: 13/13 changed files
Comments generated: 0 new

tannergooding · 2026-07-10T17:27:11Z

@MihuBot -nuget

Copilot

Copilot's findings

Files reviewed: 16/16 changed files
Comments generated: 1

…-patterns

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

EgorBo · 2026-08-04T15:08:45Z

LGTM modulo the potential bug, but to be honest, this looks like >500 LOC (not including tests) of very complex jit changes with small/questionable diffs. I think for the future we should stop doing that, instead, think about some declarative way for them. Not a big deal to postpone some peepholes like that till we get that.

tannergooding · 2026-08-04T15:19:08Z

I think for the future we should stop doing that, instead, think about some declarative way for them.

I wouldn't agree here. Getting a declarative form would be nice, but that is potentially years out if we ever get it at all.

Once we have such a thing, we can make a full pass to migrate such optimizations to it, but we shouldn't hold ourselves back and avoid improving codegen in the interim; particularly not where such patterns are standard across all 3 of the native compilers and doing so can help encourage users to utilize such standard patterns instead of writing alternatives to work around the lack of support.

As I've said before, many of these patterns sometimes have low diffs because we explicitly workaround the lack of JIT support. We can go make explicit passes to take advantage of these optimizations once they're in and see larger perf wins. -- and I expect the same goes for many of the past investigations that were closed due to "low diffs".

EgorBo · 2026-08-04T15:33:29Z

shouldn't hold ourselves back and avoid improving codegen in the interim;

I am definitely not in favor of this approach for cases with relatively small diffs and complex changes. AI now enables anyone to do this: throw 500 LOC at us for a minor gain - should we just start accepting these or maintain a bar? (By the way, we already had an enormous number of JIT bugs backported to 10.0). My opinion on peepholes is: if it's simple and easy to review, accept it even if diffs are small (but non-zero). Otherwise, there needs to be a strong justification, or it should be added to a backlog of optimizations for a future declarative system.

cc jit team @dotnet/jit-contrib for opinions.

EgorBo · 2026-08-04T15:38:14Z

Btw, I did an analysis on the .NET 10.0 jit bugs, the lower + containment part is the most popular area of regressions (6 PRs were backported to 10.0 because of bugs there).

tannergooding · 2026-08-04T15:50:04Z

AI now enables anyone to do this: throw 500 LOC at us for a minor gain - should we just start accepting these or maintain a bar?

I'm not saying there shouldn't be a bar. I in fact declared a very reasonable one, which is that if all 3 major native compilers provide it (MSVC, Clang, and GCC) then it is reasonable for us to be expected to have it as well. If only a subset of the native compilers have it, then there is less justification.

There's also, arguably, a large difference in accepting such contributions from arbitrary (especially new) community members and the people on the team who have experience in the space and optimizations involved.

The BT-family reg,reg encoding puts the value in the r/m slot and the bit index in the reg slot. Codegen worked around that by passing the operands reversed, which left the instrDesc's positional read/write info wrong: emitIsInstrWritingToReg reported the bit index as written and the value as untouched, so AreUpperBitsZero/AreUpperBitsSignExtended walked past the bts and IsRedundantMov dropped a required widening. The same reversal made emitOutputRR's GC kill target the index register. Mark them as MR-form in emitOutputRR instead, so the emitter does the swap while encoding and callers pass the natural value, index order. That also drops the pre-existing bt swap hack in genCompareInt and the operand reversal in emitDispIns. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

tannergooding · 2026-08-04T16:03:58Z

Fixed the copilot found bug and added a test, was a case where I missed that only half the support existed for bt/bts/btr/btc; in contrast to the other similar instructions that had the full handling.

Copilot

Copilot's findings

Files reviewed: 17/17 changed files
Comments generated: 0 new

tannergooding and others added 3 commits July 9, 2026 22:10

Apply formatting patch

2235f7f

Copilot AI review requested due to automatic review settings July 10, 2026 12:07

tannergooding mentioned this pull request Jul 10, 2026

Recognize bit-manipulation idioms and emit bzhi/bts/btc/btr/bt on xarch tannergooding/runtime#33

Closed

github-actions Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jul 10, 2026

Copilot started reviewing on behalf of tannergooding July 10, 2026 12:08 View session

dotnet-policy-service Bot assigned tannergooding Jul 10, 2026