feat(compiler): add EVMRangeAnalyzer dataflow pass for stack value ranges - #493
Merged
Conversation
⚡ Performance Regression Check Results✅ Performance Check Passed (interpreter)Performance Benchmark Results (threshold: 25%)
Summary: 194 benchmarks, 0 regressions ✅ Performance Check Passed (multipass)Performance Benchmark Results (threshold: 25%)
Summary: 194 benchmarks, 0 regressions |
…umbing - Move ValueRange enum to standalone header `evm_value_range.h` so both the IR builder (`Operand::ValueRange`) and `EVMAnalyzer` can share the same type. `EVMMirBuilder::ValueRange` is preserved as a `using` alias for source compatibility — no callers need to change. - Add `Operand::setRange(ValueRange)` so a downstream pass can refine the Range of an Operand after construction (used in upcoming stack-pop plumbing). - Add `BlockInfo::EntryStackRanges` (vector<EVMValueRange>) — populated by an upcoming dataflow analyzer. Empty default means "no Range info available; treat slots as ValueRange::U256" (current behavior). This is the foundation for tracking ValueRange across CFG joins; no behavior change yet. See docs/changes/2026-05-07-value-range-cfg-join/. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…anges
Add a CFG-level forward dataflow analysis on the lattice
{ U64, U128, U256 } that computes the value range of every stack slot at
each block's entry, populating BlockInfo::EntryStackRanges introduced in
Phase 1.
The pass is integrated as a final step of EVMAnalyzer::analyze and
includes:
- A per-opcode transfer function applied via linear scan over each
block's body bytes. Bitwise ops handle AND with U64 narrowing, OR/XOR
via meet-as-max; arithmetic ADD/MUL widen by one lattice step;
DIV/MOD/SHR/SAR carry the dividend or value range; LT/GT/EQ/ISZERO/
BYTE/CLZ produce U64; calls/creates produce U64 success booleans;
loads, hashes, and SUB conservatively produce U256.
- A worklist-based fixed-point: function entry seeds an empty stack,
dynamic-jump-target candidates are seeded at U256 (top) for
soundness without enumerating dynamic predecessors, and other blocks
start at U64 (bottom). Successor entry vectors are met from each
predecessor's exit state and requeued on change. The lattice height
of three guarantees O(blocks x slots x 3) convergence.
No consumer reads EntryStackRanges yet; behavior is unchanged. Phase 3
will plumb the analyzer's output into the bytecode visitor's
stackPop loop for the non-lifted JUMPDEST entry path.
Validation:
- tools/format.sh check clean
- cmake --build build --target dtvmapi succeeds with no new warnings
- evmone-unittests multipass: 223/223 (no regression)
- evmone-unittests interpreter: 215/215 (no regression)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
In `handleBeginBlock`'s non-lifted JUMPDEST path, after each `Builder.stackPop()` the popped Operand's ValueRange defaults to U256. Refine it from `BlockInfo::EntryStackRanges` (computed by the EVMRangeAnalyzer in the previous commit) so values that flow through CFG joins keep their narrow Range. This activates batch1's u64-narrow fast paths (handleBinaryArithmetic ADD/SUB, handleMul, handleDiv/handleMod, handleMulMod) on values inside loop bodies and after conditional merges — the common case for arithmetic in real Solidity contracts. Empirical sanity check (synthetic ADD u64 hot loop, 65536 ADDs/call, captured under ZEN_ENABLE_JIT_LOGGING=ON): the loop body's `ADC64rr` count drops from 3 (full 4-limb u256 ADC chain) to 0 (single ADD64rr + ICMP_ULT u64 fast path), confirming the fast path now fires across the JUMPDEST PHI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Auto-fix from `tools/format.sh format`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 3 of value-range-cfg-join introduced an unconditional Opnd.setRange() call in EVMByteCodeVisitor::handleBeginBlock() to plumb EVMRangeAnalyzer entry-stack ranges into stackPop results. The visitor is templated over IRBuilder, and the unit-test MockEVMBuilder uses MockOperand which lacked the setRange method, breaking ctest builds (ZEN_ENABLE_SPEC_TEST=ON path) — local builds without SPEC_TEST missed this. Add a no-op setRange(EVMValueRange) to MockOperand so the template instantiates cleanly. Mock tests don't exercise range-narrowed fast paths, so a no-op stub is correct.
…signed sign mismatch When dividend is U64-classified and divisor is U256-classified (potentially negative), the prior shared DIV/SDIV/MOD/SMOD transfer rule (`result = top(0)`) unsoundly classified the SDIV/SMOD result as U64. The downstream `bothFitU64` u64 fast path then truncated `limbs[2..3]` to zero, producing incorrect results for cross-CFG-join SDIV(non-neg, neg) chains. Extract `signedDivModRange` helper: returns U256 if either operand is U256 (may be negative), otherwise dividend's range (both non-negative, so signed = unsigned, |result| <= |dividend|). Strictly tighter than `meet(A, B)` -- preserves U64 classification when both inputs are U64/U128. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…TAMP/NUMBER/GASLIMIT/CHAINID) EVMC declares GetTimestamp/GetNumber/GetGasLimit as `U256Fn` returning `const intx::uint256 *` and GetChainId as `Bytes32Fn` returning a 32-byte buffer. All four can carry full uint256 values; EIP-155 chain IDs > 2^32 already exist on testnets, and future hosts may return TIMESTAMP/NUMBER values with bit 64 set. Previously the analyzer classified these as U64, which made cross-CFG-join arithmetic involving them eligible for the `bothFitU64` u64 fast path -- silently truncating high limbs. GAS (EVMC int64_t) and MSIZE (32-bit memory limit) remain genuinely U64-bounded; PC is bytecode-offset-bounded. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add Group A (per-opcode transfer rules), Group B (CFG meet convergence), Group C (dynamic-jump-target seeding), and Group D (boundary/degenerate) tests on top of the SDIV/SMOD/host-opcode regression tests from the earlier commits. ~32 new tests covering PUSH literal boundaries (U64/U128/U256 frontier exact values), arithmetic widening, bitwise short-circuit, comparison U64-pinning, dataflow meet shapes (diamond / self-loop / JUMPI asymmetry / multi-back-edge / dead block), dynamic jump target seeding, and defensive handling of truncated/undefined opcodes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…olidate docs - Replace the unreachable defensive branch in runRangeAnalysis's worklist with a ZEN_ASSERT. seedRangeEntryVectors already sizes every block with ResolvedEntryStackDepth >= 0 correctly; a silent reset to U256 hid invariant violations. - Consolidate the 7 host-context opcodes (TIMESTAMP/NUMBER/GASLIMIT/ CHAINID + BASEFEE/BLOBBASEFEE/PREVRANDAO) into a single pushTop case block in applyRangeTransferForBlock so the grouping matches the README's semantic-class table. - Move the investigation note from local docs/_archive/ into the PR's change-doc directory so it survives merge. Append a §2b-drop finding section documenting the architectural infeasibility discovered during Task 4 implementation. - Fix the per-opcode classification table to match post-fix code: U64 = PC/MSIZE/GAS/CALLDATASIZE/CODESIZE/RETURNDATASIZE; U256 = TIMESTAMP/NUMBER/GASLIMIT/CHAINID/BASEFEE/BLOBBASEFEE/PREVRANDAO. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… operands The original PR DTVMStack#493 refined u64 fast-path admission only on the non-lifted JUMPDEST path (evm_bytecode_visitor.h:1155 setRange call). Lifted blocks -- the default for well-formed bytecode -- received their entry stack values via createStackEntryOperand and materializeStackMergeOperand, both of which constructed Operands with the default ValueRange::U256. Cross-CFG-join range information was therefore unused on the dominant code path. Extend both factories to accept per-slot Range from the analyzer's BlockInfo::EntryStackRanges: - createStackEntryOperand(Range): used by EVMLiftedStackLifter::initialize to construct single-predecessor lifted entry operands. Lifter's loop has direct access to BlockInfo and slot index, so the plumbing is mechanical. - materializeLiftedBlockMergeRequests: applies setRange on the materialized PHI merge operand using BlockInfo.EntryStackRanges[Request.SlotIndex] before handing it to the StackLifter. Soundness preserved: analyzer's EntryStackRanges is a sound fixpoint (verified by 39 white-box tests in evm_range_analyzer_tests.cpp). Plumbing into lifted blocks uses the same source-of-truth, so the bothFitU64 + u128 fast-path admission invariant holds. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nt in EVMAnalyzer Review follow-ups for EVMRangeAnalyzer: - Cache `evmc_get_instruction_metrics_table` and `..._names_table` results as `EVMAnalyzer` members at construction. Removes the redundant per-call fetch+fallback dance in `analyzeBlocks` and `applyRangeTransferForBlock` (both invoked many times during analysis). - Replace the unreachable "producer exit shorter than successor entry → widen tail to U256" defensive branch in `runRangeAnalysis` with `ZEN_ASSERT(ExitStack.size() == SuccDepth)`. After `resolveEntryDepths`, every producer→successor edge has matching depths by construction; the loop had zero test coverage and silently widened on a bug rather than failing loudly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
All 39 prior analyzer tests assert `EntryStackRanges.size() == 1u`, leaving per-slot independent widening at depth ≥ 2 untested. Adds `DiamondMultiSlotMeet`: a 3-deep merge where the fallthrough predecessor exits with [U64, U128, U256] and the taken predecessor exits with [U128, U64, U64]. Per-slot `meet=max` should give [U128, U128, U256], independently widening each slot from its respective minimum across the two predecessors. Exercises pop-order indexing and lifter `Depth` math that the depth-1 tests cannot distinguish from a constant-fold. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
abmcar
force-pushed
the
perf/value-range-cfg-join
branch
from
May 12, 2026 06:41
7491a59 to
da4f4cc
Compare
abmcar
changed the title
…perf evidence - README.md: status Proposed → Implemented; tick all checklist items (Phase 1–4 done, gates green, perf measured); update Phase 3 header to reflect both non-lifted and lifted-block factory wiring; correct Findings "Future work" note now that lifted-block plumbing (2ebfd29) has landed; bump white-box test count 39 → 40. - Append "What shipped (2026-05-12)" section: 12-commit grouping, four-way perf comparison table (pre-rebase / post-rebase / D1 / lifted-plumbing), summary of decision path. - Track perf-run evidence directories from the rebase decision cycle: bench-2026-05-07/ (original PR data), perf-2026-05-11/ (pre-rebase re-run on current upstream, −1.97% geomean), perf-2026-05-11-rebased/ (post-rebase, −0.57%), perf-2026-05-11-no-483/ (D1 DTVMStack#483-revert hypothesis test, +0.95%), perf-2026-05-11-lifted/ (final lifted-block plumbing, +0.34%). Each directory has summary.md plus raw JSON + analyze.py + run_aba.sh + console captures for full reproducibility. - Track pr493-body-final.md: source-of-truth for the PR description pushed to GitHub via `gh pr edit`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… body The phrase "appears to predate this branch on current upstream" is technically wrong: the baseline IS upstream/main without this branch, so by definition snailtracer at baseline is at parity with itself; the −2.29% is produced by combining this branch with current upstream. Rebase telemetry shows pre-rebase snailtracer was +1.01% and post-rebase −7.95%, so the regression emerged after rebasing onto current upstream. Reword the three sites to honestly attribute the regression to "interaction with rebase-pickup upstream commits" while pointing out that analyzer per-opcode soundness is verified by 40 white-box tests + 2723/2723 multipass statetests, so the cause is downstream of the analyzer rather than in its classifications. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…Stack#493 Two complementary regression artifacts demonstrate the soundness fixes in commits 5d46f7e (SDIV/SMOD) and a73f782 (host-context opcodes) are not theoretical: 1. Analyzer-level white-box net (already in src/tests/evm_range_analyzer_tests.cpp). Verification on 2026-05-12: with both fixes reverted in place, 6 of the 7 directly-relevant tests fail (SDivByU256IsU256, SModByU256IsU256, TimestampIsU256, NumberIsU256, GasLimitIsU256, ChainIdIsU256); one passes by coincidence (SDivU256DividendIsU256, where pre-fix and post-fix rules agree). 2. Execution-level black-box reproducer. regression/sdiv_sign_mismatch_repro.hex is bytecode that crosses a lifted JUMPDEST and feeds the bothFitU64-gated ADD with the SDIV(U64-dividend, U256-divisor) result. regression/repro_sdiv_fast_path_truncate.sh runs it under evmone reference and DTVM multipass. Under fixed code both produce 0xFF...FC (-4 in signed 256-bit, spec-correct); under reverted code DTVM produces 0x000...000FC -- upper 192 bits truncated to 0, visible as a 32-byte RETURN data divergence. This experiment is possible only because commit 2ebfd29 plumbed the analyzer into the lifted-block codegen path. Pre-2ebfd29 the §2b analysis in investigation.md noted execution-level fixtures were architecturally infeasible -- the analyzer's setRange refinement only fired on non-lifted JUMPDESTs, so no mis-classified value could reach a fast-path consumer through the dominant codegen path. The host-context bug is harder to reproduce as a black-box test because evmc run's default host returns small values; the white-box net is the operative regression evidence for that class. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ge doc The change-doc directory should hold documentation, not raw bench data. The four perf-run directories (perf-2026-05-11, -rebased, -no-483, -lifted) and bench-2026-05-07 collectively carried ~10 MB of JSON, stdout captures, and helper scripts that are redundant once the narrative summary and four-way comparison table are in README.md. Keep only: - README.md (change spec + What-shipped section) - investigation.md (design investigation + soundness regression note) - pr493-body-final.md (PR body source-of-truth) - regression/ (the 5d46f7e soundness reproducer: README + .hex + .sh) Remove stale "Raw JSON in ..." pointers from README and PR body. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The black-box SDIV fast-path reproducer in regression/ duplicated coverage that the 40 white-box analyzer tests already provide at the CI gate. The reproducer never wired into ctest -- it relied on the local evmone install path and would have needed either path-resolution work or a C++ rewrite to be CI-friendly. Given the marginal value (catches only the narrow case where lifted-block consumer regresses to ignore analyzer ranges, which the perf bench would also surface) versus the maintenance cost, the regression/ artifacts are removed. Soundness coverage at CI time stands on the 40 white-box tests + 2723 multipass statetest fixtures. Strip the corresponding addendum from investigation.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Re-adds the SDIV soundness reproducer artifacts removed in d7f065e: regression/sdiv_sign_mismatch_repro.hex (52-byte bytecode that crosses a lifted JUMPDEST and feeds bothFitU64-gated ADD with SDIV(U64, U256) result), regression/repro_sdiv_fast_path_truncate.sh (runs evmone + DTVM multipass, asserts outputs match), and regression/README.md (bytecode walkthrough + observed-outputs table). Also restores the corresponding addendum in investigation.md. Even though the analyzer-level white-box net (40 tests in src/tests/evm_range_analyzer_tests.cpp) is the load-bearing CI gate, the black-box reproducer is worth keeping as documentation: it concretely demonstrates the user-visible state divergence (0xFF...FC -> 0x000...FC, upper 192 bits truncated) that the soundness fix prevents, and gives reviewers a one-line manual reproduction. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Single MAJOR finding from the ready-for-review preflight audit: - README.md line 132 still carried the "predates this branch on current upstream" snailtracer weasel-word that commit dc93a11 removed from the PR body. Reword to match: "appears to be an interaction with rebase-pickup upstream commits (not bisected)". Plus three follow-ups that head off reviewer confusion: - README.md line 137: clarify "12 source commits ... (last source HEAD da4f4cc; plus subsequent docs-only commits)" so the HEAD reference is not stale when the branch has had additional doc-only commits. - investigation.md header: the Light-tier fix this document originally declared "rolled back" was subsequently superseded by the deeper refactor that landed in this PR (commit 2ebfd29 lifted-block plumbing). Note that explicitly in the status field; un-mark the branch as deleted. - pr493-body-final.md: rename "Commits (12 total)" → "Source commits (12)" so reviewers counting the 18-commit diff don't wonder where the 6 docs-only commits are. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ck#493 body The body previously mentioned "40 white-box tests pass" and "2723/2723 multipass statetests pass" without showing what those tests look like under the buggy code. A reviewer reading the body cannot distinguish "the tests exist and assert correctness" from "the tests actually fire when the bug is reintroduced". Add a "Soundness regression evidence (pre-fix vs post-fix)" section between "Test plan" and "Out of scope" with two tables: 1. Analyzer-level (white-box). With both 5d46f7e and a73f782 reverted in place and rebuilt: 6 of 7 directly-relevant tests FAIL (SDivByU256IsU256, SModByU256IsU256, TimestampIsU256, NumberIsU256, GasLimitIsU256, ChainIdIsU256); one passes by coincidence (SDivU256DividendIsU256 — pre-fix and post-fix rules agree when dividend is U256). 2. Execution-level (black-box). Bytecode crossing a lifted JUMPDEST and feeding bothFitU64-gated ADD with SDIV(U64, U256-divisor): evmone reference and DTVM-with-fix both produce 0xFF...FC; DTVM with the SDIV fix reverted produces 0x000...000FC — upper 192 bits silently truncated. Note the experiment is only producible after commit 2ebfd29 plumbed the analyzer into the lifted-block codegen path, and point at the in-repo reproducer script. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a forward dataflow “range” analysis pass to the EVM frontend (EVMRangeAnalyzer) to compute per-stack-slot EVMValueRange at each block entry, and threads those ranges into both the non-lifted JUMPDEST consumer and the lifted-block operand factories to improve admission to existing u64 fast paths across CFG joins.
Changes:
- Introduce shared
EVMValueRangetype and extendEVMAnalyzer::BlockInfowithEntryStackRanges, populated via a worklist-based range analysis pass. - Refine operand ranges at non-lifted block entry (
stackPop()consumer) and during lifted-block entry/merge operand materialization. - Add a dedicated white-box unit test suite for the analyzer and wire it into the tests CMake target list; add regression documentation/artifacts.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/compiler/evm_frontend/evm_analyzer.h | Implements the EVMRangeAnalyzer worklist pass and per-opcode transfer rules; stores per-block entry stack ranges. |
| src/compiler/evm_frontend/evm_value_range.h | Adds shared EVMValueRange enum for cross-component range classification. |
| src/compiler/evm_frontend/evm_mir_compiler.h | Replaces builder-local range enum with EVMValueRange; adds Operand::setRange; updates operand factory signature to accept a range. |
| src/compiler/evm_frontend/evm_mir_compiler.cpp | Sets the requested range on newly created stack-entry operands. |
| src/compiler/evm_frontend/evm_lifted_stack_lifter.h | Threads analyzer-provided per-slot entry ranges into lifted entry operand creation. |
| src/action/evm_bytecode_visitor.h | Applies analyzer entry ranges to popped operands in non-lifted blocks; applies entry ranges to lifted merge operands. |
| src/tests/evm_range_analyzer_tests.cpp | Adds extensive white-box tests for transfer rules, CFG meet, dynamic-jump seeding, and boundary cases. |
| src/tests/evm_jit_frontend_tests.cpp | Updates mocks to satisfy new setRange/createStackEntryOperand(range) API. |
| src/tests/CMakeLists.txt | Adds and registers evmRangeAnalyzerTests executable and sanitizer/link settings. |
| docs/changes/2026-05-07-value-range-cfg-join/README.md | Design/change doc for the analyzer and its wiring. |
| docs/changes/2026-05-07-value-range-cfg-join/investigation.md | Investigation write-up and follow-up notes/regression evidence summary. |
| docs/changes/2026-05-07-value-range-cfg-join/pr493-body-final.md | Snapshot of the intended PR description text (bench + test plan). |
| docs/changes/2026-05-07-value-range-cfg-join/regression/sdiv_sign_mismatch_repro.hex | Bytecode reproducer for SDIV-related fast-path truncation under a buggy classifier. |
| docs/changes/2026-05-07-value-range-cfg-join/regression/repro_sdiv_fast_path_truncate.sh | Script to compare evmone reference vs DTVM multipass output for the SDIV reproducer. |
| docs/changes/2026-05-07-value-range-cfg-join/regression/README.md | Explanation of the regression reproducers and expected outputs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
CREATE and CREATE2 push the created contract address (20 bytes / 160 bits) or 0 on failure, not a 0/1 success boolean. The original analyzer rule grouped them with the CALL family (`CALL`/`STATICCALL`/`DELEGATECALL`/ `CALLCODE`) which DO push 0/1 success bools, classifying CREATE/CREATE2 results as U64. This is the same class of bug as the SDIV/SMOD sign-mismatch and the host-context opcode mis-classification fixed in commits 5d46f7e and a73f782: an over-claimed U64 lets the downstream bothFitU64 u64 fast path fire on a value with non-zero upper limbs, silently truncating the address. Reported by GitHub Copilot Reviewer on PR DTVMStack#493 (2026-05-12 16:17 CST). Fix: - src/compiler/evm_frontend/evm_analyzer.h: split CREATE/CREATE2 out of the U64 CALL group; classify as U256 conservatively via pushTop(). - src/tests/evm_range_analyzer_tests.cpp: add `CreateAddressIsU256` and `Create2AddressIsU256` white-box tests (suite now 42). Both follow the cross-CFG-join JUMPDEST shape used by the other soundness-class tests and fail under the pre-fix code. - docs/changes/2026-05-07-value-range-cfg-join/README.md: correct the opcode-transfer reference table -- the prior table grouped CREATE / CREATE2 with CALL and included a non-existent `RETURNDATALOAD` opcode. - docs/changes/2026-05-07-value-range-cfg-join/pr493-body-final.md and regression/README.md: bump test counts 40 -> 42 and append two rows to the pre-fix-vs-post-fix evidence table. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Round-4 review (Opus) and a user comment surfaced four classes of issues, all in the change-doc set: 1. After CREATE/CREATE2 fix (3f32828), the empirical revert claim in regression/README.md and the recipe in pr493-body-final.md still referred to "two fixes reverted at HEAD aee0e88" with the 6-of-7 FAIL table. But CreateAddressIsU256 / Create2AddressIsU256 didn't exist at aee0e88. Re-ran the empirical experiment at current HEAD with all three fixes (5d46f7e + a73f782 + 3f32828) reverted in place; 8 of 9 directly-relevant tests fail (table already shows 9 rows from the prior commit). Update recipe and HEAD reference accordingly. 2. PR body Notes section read "The two soundness commits ... close gaps where the original PR DTVMStack#493 transfer functions would have over-claimed" -- since the bugs and fixes are both in this same PR's diff, the "original PR DTVMStack#493" phrasing reads as self-modifying nonsense. Reword to describe the gaps without invoking "original PR" as a separate entity. 3. Soundness-fix grouping in both PR body and change-doc README still said "(2 commits)" -- bump to (3) and add 3f32828 to the list. 4. Test count 40 -> 42 already applied in the previous commit; this commit just propagates the empirical-revert recipe (40-test suite -> 42-test suite). No code changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three parallel review agents (code-reuse, code-quality, efficiency) surfaced six fixes worth applying. Applied five; one (BlockInfo accessor for slot indexing) skipped as marginal value vs scope. evm_analyzer.h: - EF-M1: switch the worklist `InQueue` from std::map to std::unordered_map. Cuts per-enqueue/dequeue cost from O(log N) to amortized O(1) on contracts with many blocks. - EF-M2: hoist the per-block ExitStack scratch buffer out of the worklist while-loop. Each iteration now overwrites the same vector instead of malloc'ing a fresh one; saves repeated heap traffic on pathological CFGs. - EF-m2: drop the dead `if (InstructionMetrics)` guard in the default case of applyRangeTransferForBlock. The constructor now guarantees InstructionMetrics is non-null (falls back to DEFAULT_REVISION on lookup failure). - Mi3: merge OP_ADD and OP_MUL case bodies -- they were byte-identical (both `widenRange(meetRange(A, B))`). Single case block with shared comment explaining the widening math. - M1: reword the stale comment above the SuccDepth ZEN_ASSERT. The prior text "this branch is unreachable" was leftover from when the invariant check was a defensive `if`; the new wording describes the invariant directly. No behavior change; analyzer output is identical. Gates: format clean, 42/42 white-box, 223/223 multipass evmone-unittests, 2723/2723 multipass evmone-statetest. Skipped findings (with rationale): - "Drop the BlockInfo parameter on materializeLiftedBlockMergeRequests and look up Analyzer internally" -- EVMByteCodeVisitor does not hold Analyzer as a member (it is constructed locally in run()), so the parameter is necessary. - "Lift analyzeBytecode / findBlock test helpers to a shared header" -- worth doing but a separate cleanup PR; out of scope here. - "Refactor rangeFromPushLiteral to share byte-prefix scan with AbstractValue::constFromPush" -- the two have different return types and constFromPush is on a hot path that doesn't need the U128 boundary check; sharing would create false coupling. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
abmcar
added a commit
to abmcar/DTVM
that referenced
this pull request
May 15, 2026
Add an in-source comment block in front of the perf-check Restore step so the rationale is discoverable from the workflow file directly. The bench JSON encodes runner-specific measurements; caching it across runners is what produced PR DTVMStack#493's spurious +15% interpreter "regressions". Caching the deterministic build output instead forces same-runner A-B comparison. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
abmcar
added a commit
to abmcar/DTVM
that referenced
this pull request
May 15, 2026
Promotes ~/changes/2026-05-15-perf-check-baseline-lib-cache/README.md into the project's docs/changes/ tree as part of opening the upstream PR. Empirical verification across two fork CI runs: - 25897744713 (cache miss, after re-running 3 spdlog-503 transient failures): both perf-check matrix jobs build baseline lib, then run baseline + PR bench on the same runner. 11/11 jobs green. RESULT: PASS for both modes. - 25900131271 (cache hit, same base.sha, no-op head bump): cache restored, Build-baseline-library step skipped, bench step still runs baseline + PR bench on the same runner via the dispatcher's elif branch. 11/11 jobs green on first try, no spdlog-503 retries. RESULT: PASS for both modes. Cross-runner noise reduction observed on synth/ADDRESS|BYTE|CALLER|EQ|GT|LT| ISZERO benches (PR DTVMStack#493 reported +13-18% all turned into <1% on same-runner). Only 1 micro-benchmark filtered by the baseline<5us gate versus 25 in the cross-runner cache-stale comparison. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
6 tasks
abmcar
added a commit
to abmcar/DTVM
that referenced
this pull request
May 15, 2026
The isLiteralZero ZEN_ASSERTs in the U64/U128 compare fast paths were
checking a producer-side structural invariant ("upper limbs are literal
MIR Zero constants"). PR DTVMStack#493 (EVMRangeAnalyzer cfg-join) widened the
Range contract from "literal-zero MIR" to "value-level zero" — the
analyzer now retrofits Range = U64 onto operands whose backing variables
hold any MIR that evaluates to a u64-fitting value. The old assertions
fired on analyzer-narrowed operands and crashed the JIT (CI hit SIGABRT
on stEIP1153_transientStorage.transStorageReset).
Remove the helper and the six ZEN_ASSERTs; the surrounding comments now
say "value-zero by Range contract" instead of "provably zero (literal
MIR)". The fast paths themselves are unchanged and still correct under
the new contract — they read Range and elide reading upper limbs, which
is sound for both producer-direct and analyzer-derived U64 tags.
Update the change-doc's "Invariant chain" section to reflect the two
producer paths (direct materialization + analyzer narrowing) now in
effect, instead of the old "today vs. with-cfg-join" framing.
Local validation: format clean, dtvmapi builds, 223/223 multipass
unittests pass, 2723/2723 statetest -k fork_Cancun pass.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This was referenced May 16, 2026
abmcar
added a commit
to abmcar/DTVM
that referenced
this pull request
May 24, 2026
Replace pure runtime fallback (callRuntimeFor -> intx::clz) with inline chain-select pattern modeled on handleExp's computeExpByteSize lambda (evm_mir_compiler.cpp:2511-2562). Saves HostArgScratch + Uint256ReturnBuffer roundtrip per CLZ. Tag result with ValueRange::U64 so downstream narrow consumers (PR DTVMStack#493 EVMRangeAnalyzer) can fire across basic blocks. clz(0) handled via outer Select(IsZero, 256, ...) for spec fidelity per EIP-7939, plus a defense-in-depth Limb|1 guard mirroring handleExp. Test: multipass unittests 223/223 (all 4 evm.clz_* incl. clz_osaka with 9 vectors), multipass statetest fork_Cancun 2723/2723.
abmcar
added a commit
to abmcar/DTVM
that referenced
this pull request
Jun 9, 2026
The isLiteralZero ZEN_ASSERTs in the U64/U128 compare fast paths were
checking a producer-side structural invariant ("upper limbs are literal
MIR Zero constants"). PR DTVMStack#493 (EVMRangeAnalyzer cfg-join) widened the
Range contract from "literal-zero MIR" to "value-level zero" — the
analyzer now retrofits Range = U64 onto operands whose backing variables
hold any MIR that evaluates to a u64-fitting value. The old assertions
fired on analyzer-narrowed operands and crashed the JIT (CI hit SIGABRT
on stEIP1153_transientStorage.transStorageReset).
Remove the helper and the six ZEN_ASSERTs; the surrounding comments now
say "value-zero by Range contract" instead of "provably zero (literal
MIR)". The fast paths themselves are unchanged and still correct under
the new contract — they read Range and elide reading upper limbs, which
is sound for both producer-direct and analyzer-derived U64 tags.
Update the change-doc's "Invariant chain" section to reflect the two
producer paths (direct materialization + analyzer narrowing) now in
effect, instead of the old "today vs. with-cfg-join" framing.
Local validation: format clean, dtvmapi builds, 223/223 multipass
unittests pass, 2723/2723 statetest -k fork_Cancun pass.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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
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.
Summary
Add an
EVMRangeAnalyzerdataflow pass that computes per-stack-slotValueRangeat every block entry, and wire the result into both the non-lifted JUMPDEST consumer and the lifted-block entry-operand factories. Activates the #458 u64-narrow fast paths that are actually gated onOperand::bothFitU64(handleBinaryArithmeticADD,handleMul, andhandleDiv/handleMod) on values that flow through control-flow joins — the common case for arithmetic in Solidity loop bodies.#458 introduced
Operand::ValueRangeand four families of fast paths gated onbothFitU64. Empirical investigation found these only fire intra-basic-block: at JUMPDEST entries, block-boundary stack values round-trip through factories that defaultValueRange::U256, so the fast paths rarely fired inside loop bodies in practice.This PR fixes that across both codegen paths:
EVMRangeAnalyzer): a pure dataflow pass over the EVM CFG. Lattice{U64, U128, U256}withmeet = max. Per-opcode transfer functions for the full opcode set. PopulatesBlockInfo::EntryStackRangesfor each block.evm_bytecode_visitor.h handleBeginBlocknon-lifted path): after eachstackPop(), look upBlockInfo::EntryStackRanges[slot]and refine the Operand's Range viasetRange.createStackEntryOperandtakes an explicitValueRange;materializeStackMergeOperandremains range-agnostic and the visitor refines the materialized merge operand withBlockInfo.EntryStackRanges[slot]. This is underZEN_ENABLE_EVM_STACK_SSA_LIFT(default OFF), but keeping the optional lifted path wired avoids the gap found in earlier drafts.Source commits (grouped highlights; current PR has 23 first-parent commits)
Grouped by logical role; docs/review cleanup commits are not exhaustively listed:
Analyzer infrastructure (3 commits)
3846a1e refactor(evm): extract EVMValueRange enum and prepare for analyzer plumbing— movesValueRangeto a shared header so the IR builder and analyzer share the type; addsOperand::setRangeandBlockInfo::EntryStackRanges.061c500 feat(compiler): EVMRangeAnalyzer dataflow pass for stack-slot value ranges— worklist-based fixed-point analyzer inEVMAnalyzer, with per-opcode transfer functions across the full opcode set.c6de6eb feat(evm): plumb EVMRangeAnalyzer entry ranges into stackPop consumer— non-lifted JUMPDEST consumer readsEntryStackRangesafterstackPop().Soundness fixes (3 commits, all uncovered during white-box test development or PR review)
5d46f7e fix(compiler): correct EVMRangeAnalyzer SDIV/SMOD range transfer for signed sign mismatch— analyzer was claiming U64 for SDIV/SMOD outputs whose sign-mismatch case can exceed u64 representable range.a73f782 fix(compiler): widen host-context opcode range classifications (TIMESTAMP/NUMBER/GASLIMIT/CHAINID)— these host opcodes return values whose range depends on chain state; conservative U256 widening preserves correctness on chains with non-u64 timestamps/numbers.3f32828 fix(compiler): widen CREATE/CREATE2 range to U256 in EVMRangeAnalyzer— CREATE/CREATE2 push a 20-byte contract address (or 0 on failure), not a 0/1 success bool; the original grouping with the CALL family wrongly classified the result as U64. Caught by GitHub Copilot reviewer.Tests, cleanup, lifted-block wiring (selected commits)
72c5e0b fix(test): add setRange stub to MockOperand for visitor templatee27ac3c test(compiler): EVMRangeAnalyzer white-box unit suite— 39 tests covering per-opcode transfer, CFG joins, dynamic jumps, and cross-bb chains.f203bd5 refactor(compiler): tighten EVMRangeAnalyzer defensive paths and consolidate docs— replace dead defensive branch withZEN_ASSERT; consolidate 7 host-context opcodes into single block.2ebfd29 perf(compiler): plumb EVMRangeAnalyzer ranges into lifted-block entry operands— wires the analyzer's per-slot Range into the lifted codegen path via the two Operand factories.da5571c refactor(compiler): cache instruction tables and tighten meet invariant in EVMAnalyzer— cacheevmc_get_instruction_*_tableresults as analyzer members; replace unreachable widen-tail branch withZEN_ASSERT(ExitStack.size() == SuccDepth).da4f4cc test(compiler): add multi-slot diamond meet test for EVMRangeAnalyzer— exercises per-slot independentmeet=maxat depth 3 (40th test).1dca9d5 style(evm): clang-format wrap on a Phase 3 comment blockBenchmark
evmone-bench, multipass mode, A-B-A protocol (baseline → branch → baseline_pingpong), 27
external/total/{main,micro}benches × 20 reps, taskset-pinned, single session.10d9b88da4f4ccc644fbeGitHub CI performance checks for current
10d9b88are still pending; the table above is the prior A-B-A local run atda4f4ccand should be read as manual/historical performance evidence until current CI completes.Caveat on the perf claim: the 95% lower CI is −0.07%, so the suite-level geomean improvement is not statistically distinguishable from zero at the 95% level. The original PR description claimed +1.30% (CI [+1.15%, +1.47%]) against a prior upstream/main; subsequent upstream optimizations (notably PR #483's inline arithmetic dispatch rework) have changed the interaction landscape. The lifted-block wiring fix in commit
2ebfd29recovered the analyzer-target wins onswap_math,sha1_shifts/5311,jump_around, and similar patterns (see top-wins table).snailtracer/benchmarkregresses 2.29% on this branch vs current upstream/main and is the largest single regression. The analyzer's per-opcode classifications are verified sound by 42 white-box tests and 2723/2723 multipass statetests, so the cause is how the analyzer's outputs interact with downstream codegen on the rebase-picked-up upstream commits (not yet bisected to a specific commit); deferred to a follow-up PR.Top wins
These are the per-bench patterns where the analyzer's intended fast-path activation pays off; numbers are paired-bench medians vs baseline.
micro/memory_grow_mstore/by32micro/memory_grow_mload/by16main/swap_math/insufficient_liquiditymain/sha1_divs/5311micro/jump_around/emptymain/swap_math/spentmain/sha1_shifts/5311micro/memory_grow_mload/by32micro/memory_grow_mstore/by16main/swap_math/receivedOutstanding regressions (5 / 27)
main/snailtracer/benchmarkmicro/memory_grow_mstore/by1micro/memory_grow_mload/by1main/sha1_shifts/emptymain/blake2b_huff/emptyTest plan
evmone-unittestsmultipass: 223/223evmone-unittestsinterpreter: 215/215evmone-statetest -k fork_Cancunmultipass: 2723/2723tools/format.sh checkclean10d9b88green10d9b88pendingMultipass statetest is our strongest soundness check: if any transfer function over-claims
U64on a value with non-zero upper limbs, the #458 fast paths would silently truncate and produce divergent state roots. None observed across 2723 fixtures.Soundness regression evidence (pre-fix vs post-fix)
The 42 white-box analyzer tests and 2723 multipass statetests above pass under the current code. Both fixes are also verified to be load-bearing by running with them temporarily reverted:
Analyzer-level (white-box) — revert
5d46f7e+a73f782+3f32828, rebuild, rerun the 42-test suiteSDivByU256IsU256result = Dividend = U64(wrong)SModByU256IsU256TimestampIsU256pushU64blockNumberIsU256GasLimitIsU256ChainIdIsU256CreateAddressIsU256CREATEresult as U64 (treated as success bool); it actually returns a 20-byte contract addressCreate2AddressIsU256CREATE2SDivU256DividendIsU256result = Dividend = U256happens to match the post-fix answer when dividend is U256Eight of nine directly-relevant tests fail under the pre-fix code, one passes by coincidence. The white-box net is effective at the analyzer layer.
Execution-level (black-box) —
docs/changes/2026-05-07-value-range-cfg-join/regression/A 52-byte bytecode that crosses a lifted JUMPDEST and feeds a
bothFitU64-gated ADD with the SDIV(U64-dividend, U256-divisor) result:evmone(spec reference)0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffcmode=multipasswith fix applied0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffcmode=multipasswith5d46f7ereverted0x000000000000000000000000000000000000000000000000fffffffffffffffcThe buggy output preserves only the low 64 bits of the real ADD result and zeroes the upper three limbs — exactly the "limbs[2..3] silent truncation" failure mode the fix commit message describes.
This experiment is only producible after commit
2ebfd29plumbed the analyzer's per-slot range into the lifted-block codegen path; before that commit, the analyzer'ssetRangerefinement only reached non-lifted JUMPDESTs, so no mis-classified value could reach a fast-path consumer through the dominant codegen path.Reproduce:
bash docs/changes/2026-05-07-value-range-cfg-join/regression/repro_sdiv_fast_path_truncate.shOut of scope
snailtracer/benchmarkregression bisection — analyzer per-opcode soundness verified by 42 tests + 2723/2723 statetests, but the −2.29% interaction with rebase-pickup upstream commits is not isolated to a specific commit yet; deferred to a follow-up PR.Notes
Operand::ValueRangecan trust the analyzer's classifications. The three soundness commits (5d46f7e,a73f782,3f32828) close gaps in the analyzer's transfer functions for SDIV/SMOD sign mismatch, host-context opcodes that return U256, and CREATE/CREATE2 that return contract addresses — all surfaced during this PR's white-box test development and Copilot review pass.2ebfd29resolves this and is the empirically-driven completion of the analyzer's intended consumer wiring.🤖 Generated with Claude Code