feat: rv64 alu_w shift_w#2484
feat: rv64 alu_w shift_w#2484shuklaayush merged 16 commits intorv64-word-opcode-copy-existing-modulesfrom
Conversation
|
@claude can you review the |
|
Claude finished @shuklaayush's task in 4m 40s —— View job Review of
|
|
@claude can you check if the shift operations mask the shift properly - 0x3F for rv64? also check if it was masked properly before for rv32 - 0x1F |
|
Claude finished @shuklaayush's task in 2m 55s —— View job Shift Amount Masking Analysis
Verdict: Both masks are correct, both before and after this PR. How the circuit masks the shift amountThe generic let num_bits = AB::F::from_canonical_usize(NUM_LIMBS * LIMB_BITS);
self.range_bus
.range_check(
(c[0] - limb_shift * LIMB_BITS - bit_shift) * num_bits.inverse(),
LIMB_BITS - log2(NUM_LIMBS * LIMB_BITS),
)
.eval(builder, is_valid);This decomposes The effective shift amount used by all other constraints is Constants (
|
| Constant | Value |
|---|---|
RV64_REGISTER_NUM_LIMBS |
8 |
RV64_CELL_BITS |
8 |
RV64_WORD_NUM_LIMBS |
4 |
Per-operation analysis
| Operation | Core instantiation | NUM_LIMBS × LIMB_BITS |
Circuit mask | Execution mask | RISC-V spec | Status |
|---|---|---|---|---|---|---|
| SLL/SRL/SRA (64-bit) | ShiftCoreAir<8, 8> |
64 | c[0] % 64 = & 0x3F |
rs2 & 0x3F |
rs2[5:0] | ✅ |
| SLLW/SRLW/SRAW (32-bit word) | ShiftCoreAir<4, 8> |
32 | c[0] % 32 = & 0x1F |
rs2 & 0x1F |
rs2[4:0] | ✅ |
Quotient range check validity
The quotient must fit in LIMB_BITS - log2(NUM_LIMBS × LIMB_BITS) bits:
- 64-bit shifts:
8 - log2(64) = 8 - 6 = 2bits → quotient ∈ [0, 3] → maxc[0] = 3×64 + 63 = 255✅ (full byte) - 32-bit word shifts:
8 - log2(32) = 8 - 5 = 3bits → quotient ∈ [0, 7] → maxc[0] = 7×32 + 31 = 255✅ (full byte)
Execution trace matches circuit
shift_w/execution.rs:330-347: SLLW/SRLW/SRAW users2 & 0x1F✅shift/execution.rs:334-354: SLL/SRL/SRA users2 & 0x3F✅shift/core.rs:492-498:get_shift()helper uses(y[0] as usize) % (NUM_LIMBS * LIMB_BITS)✅
Before vs. after this PR
The old shift_w/core.rs (now deleted) was an identical copy of the generic ShiftCoreAir<NUM_LIMBS, LIMB_BITS>. The masking logic was the same — this PR simply removes the duplicate. No behavioral change.
| // Write the result back to memory | ||
| exec_state.vm_write(RV32_REGISTER_AS, pre_compute.a as u32, &rd); | ||
| let rd_word = u32::from_le_bytes(<OP as ShiftWOp>::compute(rs1, rs2)); | ||
| let rd = (rd_word as i32 as i64 as u64).to_le_bytes(); |
There was a problem hiding this comment.
can we make a helper for this sign extension?
Resolves INT-6341, INT-6342. Summary: - `mul_w` and `divrem_w` reuse the existing generic core AIRs (`MultiplicationCoreAir<RV64_WORD_NUM_LIMBS, RV64_CELL_BITS>` and `DivRemCoreAir<RV64_WORD_NUM_LIMBS, RV64_CELL_BITS>`) via type aliases, eliminating the need for duplicated core code. - A new `Rv64MultWAdapter` (`mul_w.rs`) bridges 64-bit register memory with the 4-byte core interface: it reads full 8-byte registers but only passes the low 4 bytes to the core, and sign-extends the 4-byte core output back to 8 bytes on write. - The test suite is largely carried over from the original RV32 code and primarily exercises the reused core AIR logic rather than the newly written adapter. --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Ayush Shukla <shuklaayush@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
ea0a7a4
into
rv64-word-opcode-copy-existing-modules
Resolves INT-6340, INT-6343. Summary: - `base_alu_w` and `shift_w` reuse the existing generic core AIRs (`BaseAluCoreAir<RV64_WORD_NUM_LIMBS, RV64_CELL_BITS>` and `ShiftCoreAir<RV64_WORD_NUM_LIMBS, RV64_CELL_BITS>`) via type aliases, eliminating the need for duplicated core code. - A new `Rv64BaseAluWAdapter` (`alu_w.rs`) bridges 64-bit register memory with the 4-byte core interface: it reads full 8-byte registers but only passes the low 4 bytes to the core, and sign-extends the 4-byte core output back to 8 bytes on write. - The test suite is largely carried over from the original RV32 code and primarily exercises the reused core AIR logic rather than the newly written adapter. --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Ayush Shukla <shuklaayush@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Resolves INT-6340, INT-6343.
Summary:
base_alu_wandshift_wreuse the existing generic core AIRs (BaseAluCoreAir<RV64_WORD_NUM_LIMBS, RV64_CELL_BITS>andShiftCoreAir<RV64_WORD_NUM_LIMBS, RV64_CELL_BITS>) via type aliases, eliminating the need for duplicated core code.Rv64BaseAluWAdapter(alu_w.rs) bridges 64-bit register memory with the 4-byte core interface: it reads full 8-byte registers but only passes the low 4 bytes to the core, and sign-extends the 4-byte core output back to 8 bytes on write.