[RISCV] Add XReviveVec: 256-bit integers as a machine type - #7
Draft
kvpanch wants to merge 7 commits into
Draft
Conversation
EVM words are a quarter of the IR revive emits, and RISC-V has no carry flag, so the generic expansion costs 56 bytes for one `add i256` and 86 for `icmp ult`, with anything wider than 2*XLen passed by reference. Add i256 to VRM2's type list, so at VLEN=128 a wide value is an LMUL=2 vector register pair, and give it one instruction per operation in the custom-2 opcode space: arithmetic, shifts, compares, memory, conversions, and the EVM-specific addmod/mulmod/exp/signextend. Wide arguments pass in vector registers. Copies and spills reuse the existing RVV paths. Supporting changes: raise MaxDivRemBitWidthSupported so ExpandIRInsts leaves wide div/rem alone; guard the seteq/setne complex pattern against non-XLen operands; reserve FP up front, since wide spill slots appear after hasFP is decided; refuse to merge constant stores into a 256-bit one, which does not terminate; add IIT_I256 so intrinsics can take a 256-bit scalar. Over revive's 15 benchmark contracts (103 modules): -30.04% code, -25.96% including the constant pool.
kvpanch
marked this pull request as draft
August 14, 2026 11:41
Member
|
Quick thought about it, should we make the CSR_V registers callee-saved instead? Since they aren't used for arg passing, it may be beneficial |
EVM words are 256 bits and RISC-V registers 64, so every wide operation becomes a four-limb chain, each carry costing an `sltu` to produce and another to consume, with anything wider than 2*XLen passed by reference. Add i256 to VRM2, so at VLEN=128 a wide value is an LMUL=2 register pair, and give it one instruction per operation in the custom-2 opcode space. The feature pins VLEN, which also makes RVV spill slots fixed-size rather than sized in vlenb. Over revive's 15 benchmark contracts (103 modules), .text: V5QuoteVerifier 564,202 -> 328,998 -41.7% XENCrypto 469,344 -> 235,854 -49.8% FiatTokenV1 356,650 -> 240,108 -32.7% SnapshotPCCSRouter 296,694 -> 176,628 -40.5% AutomataDcapAttestationFee 281,214 -> 170,824 -39.3% Festival 181,228 -> 122,298 -32.5% P2PMarket 135,070 -> 74,938 -44.5% T3rminalDriver 133,062 -> 72,018 -45.9% TetherToken 50,432 -> 32,256 -36.0% ERC20Factory 24,420 -> 16,030 -34.4% DaimoP256Verifier 21,388 -> 9,280 -56.6% ERC20 21,044 -> 12,860 -38.9% Multicall3 20,854 -> 13,584 -34.9% WETH9 19,402 -> 14,200 -26.8% Sha256 7,354 -> 6,220 -15.4% TOTAL 2,582,358 -> 1,526,096 -40.9% -36.5% including the constant pool. Opt-in and off by default.
Author
Perhaps, we can change it too, but this will break RISC-V vector ABI |
elle-j
reviewed
Aug 17, 2026
Comment on lines
+1989
to
+2006
| let TargetPrefix = "riscv" in { | ||
| def int_riscv_revive_addmod | ||
| : DefaultAttrsIntrinsic<[llvm_i256_ty], | ||
| [llvm_i256_ty, llvm_i256_ty, llvm_i256_ty], | ||
| [IntrNoMem, IntrSpeculatable]>; | ||
| def int_riscv_revive_mulmod | ||
| : DefaultAttrsIntrinsic<[llvm_i256_ty], | ||
| [llvm_i256_ty, llvm_i256_ty, llvm_i256_ty], | ||
| [IntrNoMem, IntrSpeculatable]>; | ||
| def int_riscv_revive_exp | ||
| : DefaultAttrsIntrinsic<[llvm_i256_ty], | ||
| [llvm_i256_ty, llvm_i256_ty], | ||
| [IntrNoMem, IntrSpeculatable]>; | ||
| def int_riscv_revive_signextend | ||
| : DefaultAttrsIntrinsic<[llvm_i256_ty], | ||
| [llvm_i256_ty, llvm_i256_ty], | ||
| [IntrNoMem, IntrSpeculatable]>; | ||
| } |
There was a problem hiding this comment.
Could we note the operand order here as comments? The definitions carry types only, and since all are i256 in every position there is nothing to really distinguish them.
To match what revive's (replaced) stdlib.ll used, I'd suggest something like this (// Input: seems to be the convention in this file):
// Input: (a, b, modulus)
def int_riscv_revive_addmod : ...
// Input: (a, b, modulus)
def int_riscv_revive_mulmod : ...
// Input: (base, exponent)
def int_riscv_revive_exp : ...
// Input: (byte_index, value)
def int_riscv_revive_signextend : ...
This order is also what I note in revive PR 595.
Add i512 to VRM4 (LMUL=4) and a new i1024 machine type to VRM8 (LMUL=8), so the same instruction set exists at three widths, generated from one multiclass. The operation encoding has no room for a width -- the I- and S-type memory forms have no spare field -- so each width takes its own custom opcode: i256 in custom-2, i512 in custom-3, i1024 in custom-1. Also adds wide min/max, and the cross-width truncation and zero-extension the single-width version had no need for. The corpus contains eight i512 operations, so it barely moves. On an artificial benchmark of the same work at each width, .text: i256 3,404 -> 88 -97.4% i512 10,886 -> 88 -99.2% i1024 45,328 -> 118 -99.7% The expansion is superlinear in limb count while the instruction sequence is flat.
Three crashes and a broken test found reviewing the last two commits:
select i512/i1024 the width commit added the register classes but only
Select_VRM2, so SELECT_CC at the wider LMULs could not
be selected
narrowing mul emitted scalar MUL without a Zmmul guard, so
-mattr=+xrevivevec alone aborted in the asm printer
VLEN diagnostic tested whether the flag was passed rather than the
resolved bounds, so vscale_range(4,4) set VLEN=256
unchecked; it also rejected bits-min=0, which upstream
means "derive from Zvl"
disasm test wrapped llvm-mc in `not` for the no-extension run, but
decoding an unknown encoding warns and exits 0
The wider integers had an instruction set each, in custom-2, custom-3 and custom-1, so three quarters of the custom encoding space went to a width axis -- including the space reserved for the carry extension. LMUL is a vtype field precisely so instructions need no per-width encoding: one `revive.wadd` now covers 256, 512 and 1024 bits, whichever `vsetivli` is in force, and the whole extension fits in custom-2. The encodings carry no width and take single vector registers. Per-width pseudos carry the register class and LMUL, so the allocator sees the group and RISCVInsertVSETVLI emits the configuration and elides the redundant ones; they lower back to the one encoding through RISCVVPseudosTable. `BaseInstr` is named rather than derived, because PseudoToVInst strips `_SE` and would otherwise map REVIVE_W_SEQ to REVIVE_WQ. A wide load or store carries a relocation on its offset, which no standard vector pseudo does, so the vector pseudo lowering falls back to the ordinary operand lowering instead of asserting. Re-establishing vtype after every call cost 4.6% of .text, so `vl` and `vtype` are call-preserved in the E ABI. The register mask is what tells the insertion pass the configuration is gone, so gating its `isCall` checks alone changed nothing; the mask is used for preservation only, and neither register gets a frame slot. The obligation is on the callee, and it holds for what revive emits, where every wide operation is LMUL=2. -39.06% code against no extension over the 103 benchmark modules, against -39.94% for the three-opcode design: the width in vtype costs +4.65% and the call-preserved vtype gives back 68% of it.
The width had come from vtype's LMUL, which meant a consumer of the object could not tell how wide an instruction was without following the control flow to find the configuration in force. funct7 had the room, so it goes there: funct7[6:5] is the width and funct7[4:0] the operation, with the R-type shapes sharing one funct3 the way the base ISA does. Loads keep a full twelve-bit immediate, one funct3 each, so a constant pool entry is still reachable through %lo; a store never needs one and gives up two bits instead. Everything stays in custom-2, one funct3 is spare, and no vsetivli is emitted at all -- which also removes the vtype machinery this had accumulated: the per-block configuration, the call-preserved vl/vtype, and the register move that read its width from vtype and so copied half a value. Four places treat a vector copy as demanding vtype, not one; all are gated, since revive.mvN carries the count it moves. Fixed-length vectors are off and the oversized-equality combine bails, so nothing selects a standard vector instruction PolkaVM does not implement.
…reserve it LMUL is a vtype field precisely so instructions need no per-width encoding: one revive.wadd again covers 128 through 1024 bits, whichever vsetivli is in force, and the whole extension fits in custom-2 with custom-1 and custom-3 left free. Three pieces of the funct7 work are orthogonal to how width is carried and stay: register moves that name their own count, since a copy expands after the vsetvli pass and can never read vtype; the fixes that stop RVV codegen leaking in; and spills through the extension's own load and store. The outliner may no longer lift instructions that read vtype. It runs after vsetvli insertion, so an outlined body's width becomes a property of the call site, which no consumer can recover from the instruction stream. hasCallPreservedVType() no longer returns true unconditionally. Callers assumed vtype survived a call while nothing restored it: the CalleeSavedRegs entry never took effect, VTYPE being reserved and so never spilled. A caller at m2 calling a callee that configured m4 resumed at m4, reloading 64 bytes where 32 were spilled.
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
3 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.
EVM words are a quarter of the IR revive emits, and RISC-V has no carry flag, so the generic expansion costs 56 bytes for one
add i256and 86 foricmp ult, with anything wider than 2*XLen passed by reference.Add i256 to VRM2's type list, so at VLEN=128 a wide value is an LMUL=2 vector register pair, and give it one instruction per operation in the custom-2 opcode space: arithmetic, shifts, compares, memory, conversions, and the EVM-specific addmod/mulmod/exp/signextend. Wide arguments pass in vector registers. Copies and spills reuse the existing RVV paths.
Supporting changes: raise MaxDivRemBitWidthSupported so ExpandIRInsts leaves wide div/rem alone; guard the seteq/setne complex pattern against non-XLen operands; reserve FP up front, since wide spill slots appear after hasFP is decided; refuse to merge constant stores into a 256-bit one, which does not terminate; add IIT_I256 so intrinsics can take a 256-bit scalar.
Over revive's 15 benchmark contracts (103 modules): -30.04% code, -25.96% including the constant pool.