perf(compiler): merge linear memory prechecks for direct-memory recur…#436
Open
ECNUyhy wants to merge 2 commits intoDTVMStack:mainfrom
Open
perf(compiler): merge linear memory prechecks for direct-memory recur…#436ECNUyhy wants to merge 2 commits intoDTVMStack:mainfrom
ECNUyhy wants to merge 2 commits intoDTVMStack:mainfrom
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 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a new “linear recurrence” memory precheck plan in the EVM MIR frontend to merge per-op memory expansion checks for repeated direct-memory MLOAD/MSTORE motifs (especially those driven by a stride), and introduces new EVM asm samples intended to exercise the optimization/regression cases.
Changes:
- Add a new block-level linear memory precheck plan, with emission/consumption logic in
EVMMirBuilder(and new compile stats counters). - Extend
EVMByteCodeVisitorblock analysis to detect specific linear-recurringMLOAD/MSTOREbytecode motifs and feed the plan into the builder. - Add new
tests/evm_asm/*samples/expected outputs for linear memory motifs and a const-precheck regression case.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/compiler/evm_frontend/evm_mir_compiler.h |
Declares linear precheck plan APIs/state and new compile stats counter fields. |
src/compiler/evm_frontend/evm_mir_compiler.cpp |
Implements linear precheck planning/emission and wires it into handleMLoad/handleMStore; extends logging/stats. |
src/action/evm_bytecode_visitor.h |
Adds bytecode motif scanning for linear direct-memory recurrences and calls into builder to prepare/emit the precheck. |
tests/evm_asm/memory_linear_mstore_step.easm |
New asm sample intended to match the linear MSTORE motif with a final RETURN. |
tests/evm_asm/memory_linear_mstore_step.expected |
Expected result for the above sample. |
tests/evm_asm/memory_linear_mstore_hit.easm |
New asm sample intended to match the linear MSTORE motif ending with STOP. |
tests/evm_asm/memory_linear_mstore_hit.expected |
Expected result for the above sample. |
tests/evm_asm/memory_linear_mload_step.easm |
New asm sample intended to match the linear MLOAD motif with a final RETURN. |
tests/evm_asm/memory_linear_mload_step.expected |
Expected result for the above sample. |
tests/evm_asm/memory_linear_mload_hit.easm |
New asm sample intended to match the linear MLOAD motif ending with STOP. |
tests/evm_asm/memory_linear_mload_hit.expected |
Expected result for the above sample. |
tests/evm_asm/memory_const_precheck_regress.easm |
New asm sample for const-precheck regression coverage. |
tests/evm_asm/memory_const_precheck_regress.expected |
Expected result for the above sample. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1
to
+3
| PUSH1 0x00 | ||
| CALLDATALOAD | ||
| PUSH1 0x00 |
There was a problem hiding this comment.
This test’s stride is sourced from CALLDATALOAD, but the current evm_asm harness executes with input_size = 0, so the loaded stride will always be 0. That means the test won’t exercise the non-zero stride/overflow behavior of the new linear-precheck logic (it mostly becomes a degenerate same-address sequence). Consider extending the harness to allow providing calldata for these .hex cases (or adding an alternative JIT test that can supply calldata) so this pattern is actually validated.
Comment on lines
+1
to
+4
| PUSH1 0x00 | ||
| CALLDATALOAD | ||
| PUSH1 0x00 | ||
|
|
There was a problem hiding this comment.
This test’s stride is sourced from CALLDATALOAD, but the current evm_asm harness executes with input_size = 0, so the loaded stride will always be 0. That means the test won’t exercise the non-zero stride/overflow behavior of the new linear-precheck logic (it mostly becomes a degenerate same-address sequence). Consider extending the harness to allow providing calldata for these .hex cases (or adding an alternative JIT test that can supply calldata) so this pattern is actually validated.
Suggested change
| PUSH1 0x00 | |
| CALLDATALOAD | |
| PUSH1 0x00 | |
| PUSH1 0x20 | |
| PUSH1 0x00 |
Comment on lines
+3183
to
+3189
| if (!tryConsumeLinearBlockMemoryPrecheck(Offset, nullptr) && | ||
| !tryConsumeConstBlockMemoryPrecheck()) { | ||
| MInstruction *SizeConst = createIntConstInstruction(I64Type, 32); | ||
| MInstruction *RequiredSize = createInstruction<BinaryInstruction>( | ||
| false, OP_add, I64Type, Offset, SizeConst); | ||
| MInstruction *Overflow = createInstruction<CmpInstruction>( | ||
| false, CmpInstruction::Predicate::ICMP_ULT, I64Type, RequiredSize, |
There was a problem hiding this comment.
The new linear block memory precheck path isn’t covered by any test that actually runs the EVM frontend compiler/JIT. The added tests/evm_asm/* cases are executed via the interpreter harness, so they won’t detect regressions in tryConsumeLinearBlockMemoryPrecheck()/setMemoryCompileBlockLinearPrecheckPlan(). Please add a JIT/multipass-mode test that compiles and executes one of these patterns and asserts the behavior that should change (e.g., only a single memory expand/check emitted for the block, or the related compile stats/log counters).
Comment on lines
+1
to
+3
| PUSH1 0x00 | ||
| CALLDATALOAD | ||
| PUSH1 0x00 |
There was a problem hiding this comment.
This test’s stride is sourced from CALLDATALOAD, but the current evm_asm harness executes with input_size = 0, so the loaded stride will always be 0. That means the test won’t exercise the non-zero stride/overflow behavior of the new linear-precheck logic (it mostly becomes a degenerate same-address sequence). Consider extending the harness to allow providing calldata for these .hex cases (or adding an alternative JIT test that can supply calldata) so this pattern is actually validated.
Comment on lines
+1
to
+3
| PUSH1 0x00 | ||
| CALLDATALOAD | ||
| PUSH1 0x00 |
There was a problem hiding this comment.
This test’s stride is sourced from CALLDATALOAD, but the current evm_asm harness executes with input_size = 0, so the loaded stride will always be 0. That means the test won’t exercise the non-zero stride/overflow behavior of the new linear-precheck logic (it mostly becomes a degenerate same-address sequence). Consider extending the harness to allow providing calldata for these .hex cases (or adding an alternative JIT test that can supply calldata) so this pattern is actually validated.
Suggested change
| PUSH1 0x00 | |
| CALLDATALOAD | |
| PUSH1 0x00 | |
| PUSH1 0x20 | |
| PUSH1 0x00 |
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.
…rences
1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):
2. What is the scope of this PR (e.g. component or file name):
3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):
4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):
5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links:
6. Release note