[U3-B] comparisons and arithmetic shift on the carrier dispatch - #28
Merged
EdanStasiuk merged 7 commits intoJul 21, 2026
Merged
Conversation
Extends the Nybbler pass to support arithmetic right shifts (AShr) and comparisons (ICmp: eq, ne, ult, slt) for narrow-field (i1/i2/i4) vectors. Need to iron out some kinks, got a couple failed tests.
EdanStasiuk
marked this pull request as draft
July 19, 2026 08:39
lowerUlt computed the per-field borrow bit as `Raw ^ b ^ ~a`, but this
is algebraically identical to `Raw ^ a ^ ~b` (since a^~b == ~a^b for
any bits), so no XOR-only combination of {Raw, a, b, ~a, ~b} can
distinguish a <u b in general -- the formula was mathematically unable
to be correct.
Replace it with the borrow-out formula derived from the containment
trick (a's top bit forced 1, b's top bit forced 0):
a <u b == (~a & b) | (~(a^b) & ~Raw) [top bit of each field]
Verified by exhaustive brute-force check over all i2/i4 field values.
lowerSlt reuses lowerUlt via the sign-flip trick, so it is fixed as a
side effect -- no changes needed there.
Fixes diff/ult.ll and diff/slt.ll.
EdanStasiuk
marked this pull request as ready for review
July 20, 2026 08:08
diff/ashr.ll failed intermittently in CI but not locally, e.g.: MISMATCH ashr_i4 a=aaaa...a b=aaaa...a ref=eeee...e cand=ffff...f Each i4 field of `b` is 0b1010 = 10, which is >= N (4) -- an out-of-range shift amount, which is poison per the LLVM LangRef. The harness's reference runs the *unlowered* op through lli, so for a poison input its result depends on the host LLVM's legalization, which differs between local (Homebrew llvm@22.1.8, macOS/arm64) and CI (apt llvm-22, Linux/x86_64). The candidate (Nybbler's carrier lowering) is deterministic and always saturates to sign, so only `ref` diverged -- a harness gap, not a lowering bug. Add mask_shift_amounts(), clamping each N-bit field of a shift kernel's `b` operand into [0, N-1], applied in build_module() for shl_/lshr_/ashr_ kernels. `a` is untouched so data values are still fully exercised. No changes to Nybbler.cpp: lowerAshr's saturating behavior for out-of-range amounts is intentional and doesn't need to match poison output, since poison has no single correct value.
angelo-yap
approved these changes
Jul 20, 2026
EdanStasiuk
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes: #7
Summary
Extends the Nybbler pass to support arithmetic right shifts (AShr) and comparisons (ICmp: eq, ne, ult, slt) for narrow-field (i1/i2/i4) vectors.
Adds three new handler families to the carrier dispatch (
getHandler), each registered per the existingCarrierHandlerpattern:icmp eq/ne--lowerEqNe: xor + within-field OR-reduce/broadcast (reduceAndBroadcastField), degenerating correctly at i1.icmp ult/slt--lowerUlt/lowerSlt: SWAR unsigned compare via borrow-containment subtraction (ult.ahi/ult.blo/ult.raw), withsltimplemented as a sign-bit flip ahead of the same unsigned compare. i1 ult/slt are trivial and special-cased to single-bit logic (~a & b,a & ~b) before dispatch.ashr--lowerAshr: same barrel-shift skeleton asshl/lshr(lowerShift), but vacated high bits are filled with each field's sign bit instead of zero. i1 is special-cased to identity.Two real bugs were found and fixed during implementation:
lowerUlt's borrow formula (Raw ^ b ^ ~a) was algebraically incapable of computing unsigned less-than for any input, sincea^~b == ~a^bcollapses it to one XOR expression regardless of operand order. Replaced with the correct borrow-out formula,(~a & b) | (~(a^b) & ~Raw), verified by exhaustive brute-force check over all i2/i4 field values.lowerSltwas fixed as a side effect, since it reuseslowerUltvia the sign-flip trick.diff/ashr.llfailed intermittently in CI but not locally. Root cause was in the test harness, not the pass: for shift kernels, theboperand doubles as a per-field shift amount, and the generated trials included out-of-range amounts (>= N), which are poison per the LLVM LangRef. The harness's "reference" runs the unlowered op throughlli, so for a poison input its result depends on the host LLVM's legalization -- which differed between local (Homebrew llvm@22.1.8, macOS/arm64) and CI (apt llvm-22, Linux/x86_64) -- while Nybbler's deterministic carrier lowering stayed consistent across both. Fixed by clamping each field of the shift-amount operand into[0, N-1]indiff_runner.pybefore it's emitted, so no trial exercises a poison shift amount.Tests
eq/ne/ult/slt/ashr(i1/i2/i4). Several had staleCHECKlines left over from before these ops were implemented (e.g. expecting a literalicmp eq/icmp ult/ashron the carrier, when the actual lowering never emits those opcodes directly -- it's built fromxor/and/or/sub/lshr/shl); updated each to check for the real emitted instruction sequence instead.diff/eq.ll,diff/ne.ll,diff/ult.ll,diff/slt.ll,diff/ashr.llall pass, confirming correctness against the scalar reference across structured edge cases and randomized trials.