U2-B Add swar sub and shift masks

[EdanStasiuk]

Summary

Completes the arithmetic and shift lowerings on the carrier dispatch path.

Sub (i4 / i2) uses a borrow-absorber approach, the dual of lowerAdd:

1. Set the top bit of a in every field (OR hmask) so borrow has somewhere to go without escaping the field boundary
2. Clear the top bit of b (AND lmask) and subtract
3. Fix up the top bits: strip the artificial 1 (XOR hmask), then XOR in the correct a_top ^ b_top

i1 sub remaps to xor before dispatch, mirroring the existing i1 add remap.

Shl / LShr (i4 / i2) replaces the U2-A identity-mask placeholder with a real per-field bit-serial conditional shift. For each power-of-two step s:

• Extract whether each field's amount has bit s set
• Conditionally apply a carrier shift by s with the correct boundary mask (top N-s bits for shl, bottom N-s bits for lshr)
• Blend shifted and unshifted values per field

Shift amounts are masked to [0, N-1] per field inside the handler so out-of-range values produce defined output consistent with the scalar reference in the differential harness.

i1 shl / lshr return the input unchanged — the only in-range amount is 0 (identity), any nonzero is poison.

Tests

• Removed XFAIL: * from test/shape/sub_i1.ll, sub_i2.ll, sub_i4.ll
• Updated CHECK lines in all sub shape tests to match the real borrow-absorber sequence (or/and/sub/xor/xor) and the i1 xor remap
• Removed XFAIL: * from test/shape/shl_i1/i2/i4.ll and lshr_i1/i2/i4.ll
• Updated CHECK lines in all shl/lshr shape tests to match the bit-serial sequence (and/lshr/and/sub/shl-or-lshr/and) and the i1 identity path
• Removed XFAIL: * from test/diff/shl.ll and test/diff/lshr.ll

[Copilot]

Pull request overview

Implements missing carrier-path lowerings in Nybbler for narrow-field vector sub and variable shl/lshr, and re-enables associated shape/differential tests that were previously XFAILed.

Changes:

• Add SWAR sub lowering for i2/i4 and remap i1 sub to xor before dispatch.
• Replace shift scaffolding with per-field bit-serial conditional shift lowering for shl/lshr on i2/i4 and an identity fast-path for i1.
• Update shape tests’ CHECK patterns and remove XFAILs from sub/shl/lshr shape + diff tests.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
lib/Nybbler.cpp	Adds lowerSub, extends dispatch remaps for i1 sub, and implements per-field conditional shl/lshr lowering.
test/shape/sub_i4.ll	Updates shape CHECKs for the new borrow-confining SWAR sub sequence; removes XFAIL.
test/shape/sub_i2.ll	Updates shape CHECKs for the new borrow-confining SWAR sub sequence; removes XFAIL.
test/shape/sub_i1.ll	Updates shape CHECKs to validate i1 sub remaps to xor; removes XFAIL.
test/shape/shl_i4.ll	Updates shape CHECKs for the new bit-serial per-field shift sequence; removes placeholder assumptions.
test/shape/shl_i2.ll	Updates shape CHECKs for the new bit-serial per-field shift sequence; removes placeholder assumptions.
test/shape/shl_i1.ll	Updates shape CHECKs to validate i1 shl lowers to an identity carrier path (no shl).
test/shape/lshr_i4.ll	Updates shape CHECKs for the new bit-serial per-field shift sequence; removes placeholder assumptions.
test/shape/lshr_i2.ll	Updates shape CHECKs for the new bit-serial per-field shift sequence; removes placeholder assumptions.
test/shape/lshr_i1.ll	Updates shape CHECKs to validate i1 lshr lowers to an identity carrier path (no lshr).
test/diff/shl.ll	Removes XFAIL now that shl lowering is expected to match the scalar reference.
test/diff/lshr.ll	Removes XFAIL now that lshr lowering is expected to match the scalar reference.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[angelo-yap]

CI is red on 4 tests — two real correctness bugs in lowerShift, one cosmetic

diff/lshr.ll   diff/shl.ll        <- wrong results (differential harness vs LLVM reference)
shape/lshr_i2.ll  shape/shl_i2.ll <- FileCheck sequence mismatch

The diff/* failures are the important ones — they mean the lowered code computes the wrong value, not just a mismatched CHECK. I traced both bugs and confirmed the reference semantics against lli.

Bug 1 — per-field select mask is built with a carrier-wide subtract (wrong for packed fields)

lib/Nybbler.cpp (lines ~199-201):

Value *StepSel = B.CreateAnd(
    B.CreateSub(Zero, HasStep, "shift.sel_raw"),   // <-- one i8 subtract
    FieldMask, "shift.sel");

The (0 - bit) & fieldmask trick expands 0/1 -> 0x00/all-ones, but only for a single field in a register. Here the carrier packs multiple fields per byte, so the subtract's borrows bleed across field boundaries.

Example (i2, all fields want the step, HasStep = 0x55):

• want per-field mask 0xFF (each field 0b11)
• 0 - 0x55 = 0xAB = 0b10101011 → only the lowest field becomes 0b11; the rest are 0b10.

This corrupts the blend, so even in-range shifts are wrong. Hand-tracing shl_i2 a=ffff b=ffff with this mask reproduces the cand=fefe in the CI log exactly.

Fix direction: build the mask without cross-field carries. Since HasStep only sets bit 0 of each field, smear it up within the field with in-field shifts (mask |= mask << k for k = 1,2,…< N) — bit 0 shifted by < N stays inside its field, so no bleed and no masking needed.

Bug 2 — over-shift (amount ≥ N) is not handled

Lines ~172-174 mask the amount into range:

unsigned AmtBits = (N == 2) ? 1 : 2;
Value *Amt = B.CreateAnd(Ops[1], splatFieldPattern(...), "shift.amt");

and the barrel loop only runs for (S = 1; S < N; S <<= 1). So an i2 amount of 2 or 3 is treated as a shift of 0/1.

But the differential harness' reference is LLVM's own scalarized shift, which over-shifts to 0. Confirmed on lli:

shl i2 3, 2  ->  0
shl i2 3, 3  ->  0

That's why e.g. shl_i2 a=ffff b=aaaa gives ref=0000 cand=ffff. The N==1 → return Ops[0] early-return has the same flaw (shl i1 x, 1 should be 0, not x).

Fix direction: don't pre-mask the amount. Use each field's full N-bit value and extend the barrel loop to cover every amount bit (S = 1 … 2^(N-1)). A step with S >= N clears the field (its boundary mask is empty), which naturally yields 0 for over-shift and also makes i1 fall out of the general path — the N==1 shortcut can be removed.

shape/shl_i2.ll + shape/lshr_i2.ll (cosmetic)

These CHECK an lshr <2 x i8> (line 10) that the i2 path never emits — the amount-extraction lshr only appears for i4. Looks like the i2 shape files were copied from the i4 template. Once the lowering is fixed, regenerate/adjust these to match the actual i2 output. (shape/shl_i4.ll + shape/lshr_i4.ll will also need updating since the instruction sequence changes.)

Happy to put up a patch for lowerShift implementing both fix directions above (and regenerating the shape tests) if that's useful.