feat(compiler): add EVM memory compile instrumentation summary#414
Merged
zoowii merged 5 commits intoDTVMStack:mainfrom Mar 26, 2026
Merged
feat(compiler): add EVM memory compile instrumentation summary#414zoowii merged 5 commits intoDTVMStack:mainfrom
zoowii merged 5 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
Adds a lightweight instrumentation summary for EVM memory-related MIR compilation, emitting counters that help diagnose how often memory expansion/size reload/pointer fetch logic is generated during compilation.
Changes:
- Introduces
EVMMirBuilder::MemoryCompileStatscounters and increments them at key memory-compilation sites. - Adds
EVMMirBuilder::dumpMemoryCompileStats()to emit a single summary log line when counters are non-zero. - Calls the dump hook after EVM MIR compilation completes.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/compiler/evm_frontend/evm_mir_compiler.h | Declares stats struct/counters and the dumpMemoryCompileStats() API on EVMMirBuilder. |
| src/compiler/evm_frontend/evm_mir_compiler.cpp | Increments stats counters and implements hasMemoryCompileStats() + summary debug logging. |
| src/compiler/evm_compiler.cpp | Invokes dumpMemoryCompileStats() after MIR building. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+4065
to
+4075
| ZEN_LOG_DEBUG( | ||
| "[EVM-MEM-SUMMARY] mload_expand=%llu mstore_expand=%llu " | ||
| "mstore8_expand=%llu mcopy_expand=%llu reload_mem_size=%llu " | ||
| "get_mem_ptr=%llu need_expand_cfg=%llu", | ||
| static_cast<unsigned long long>(MemStats.MLoadExpandCount), | ||
| static_cast<unsigned long long>(MemStats.MStoreExpandCount), | ||
| static_cast<unsigned long long>(MemStats.MStore8ExpandCount), | ||
| static_cast<unsigned long long>(MemStats.MCopyExpandCount), | ||
| static_cast<unsigned long long>(MemStats.ReloadMemorySizeCount), | ||
| static_cast<unsigned long long>(MemStats.GetMemoryDataPointerCount), | ||
| static_cast<unsigned long long>(MemStats.ExpandNeedExpandCFGCount)); |
Comment on lines
46
to
+47
| MIRBuilder.compile(&Ctx); | ||
| MIRBuilder.dumpMemoryCompileStats(); |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds block-level EVM memory compilation diagnostics and introduces a first-wave block-local memory precheck optimization for eligible “pure direct-memory” basic blocks, aiming to reduce redundant per-op memory expansion CFG.
Changes:
- Adds memory compile summary + per-basic-block memory instrumentation in
EVMMirBuilder. - Implements a block-local const precheck plan to hoist a single
expandMemoryIR()for eligible blocks and skip per-op expansion checks. - Extends the bytecode visitor with a conservative block scan to detect constant-address direct-memory patterns and emit precheck plans.
Reviewed changes
Copilot reviewed 8 out of 8 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 memory compile stats structs and block-precheck APIs/states. |
| src/compiler/evm_frontend/evm_mir_compiler.cpp | Implements stats logging, block tracking, and const precheck emission/consumption. |
| src/compiler/evm_compiler.cpp | Dumps memory compile summary after MIR build when logging is enabled. |
| src/action/evm_bytecode_visitor.h | Adds block scanning for const-address direct-memory blocks and hooks for block begin/end + opcode notes. |
| openspec/changes/update-evm-memory-block-precheck/tasks.md | Tracking/tasks doc for the new optimization work. |
| openspec/changes/update-evm-memory-block-precheck/specs/evm-jit/spec.md | Spec requirements for the block-local precheck behavior and diagnostics. |
| openspec/changes/update-evm-memory-block-precheck/proposal.md | Proposal/motivation and scope for the optimization. |
| openspec/changes/update-evm-memory-block-precheck/design.md | Design/eligibility/risks/validation plan for the optimization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+4258
to
+4260
| MInstruction *RequiredSize = createIntConstInstruction( | ||
| I64Type, CurBlockConstPrecheckPlan.MaxRequiredSize); | ||
| MInstruction *NoOverflow = createIntConstInstruction(I64Type, 0); |
There was a problem hiding this comment.
expandMemoryIR(RequiredSize, Overflow) is invoked elsewhere with Overflow coming from a CmpInstruction (i.e., a boolean/i1). Here, NoOverflow is created as an i64 constant, which is very likely a type mismatch for the branch/condition inside expandMemoryIR and can break verification or generate incorrect IR. Use an i1 false constant (or the project’s boolean-const helper) for the overflow argument.
Suggested change
| MInstruction *RequiredSize = createIntConstInstruction( | |
| I64Type, CurBlockConstPrecheckPlan.MaxRequiredSize); | |
| MInstruction *NoOverflow = createIntConstInstruction(I64Type, 0); | |
| MType *I1Type = &Ctx.I1Type; | |
| MInstruction *RequiredSize = createIntConstInstruction( | |
| I64Type, CurBlockConstPrecheckPlan.MaxRequiredSize); | |
| MInstruction *NoOverflow = createIntConstInstruction(I1Type, 0); |
src/action/evm_bytecode_visitor.h
Outdated
| } | ||
|
|
||
| static bool isBlockTerminatorOpcode(evmc_opcode Opcode) { | ||
| return Opcode == OP_JUMP || Opcode == OP_RETURN || Opcode == OP_STOP || |
There was a problem hiding this comment.
OP_JUMPI is a basic-block terminator in EVM. Omitting it means analyzeConstDirectMemoryBlockPrecheck(...) can scan past the end of the current basic block into the fallthrough block (which does not necessarily start with JUMPDEST), potentially (a) incorrectly disqualifying eligible blocks, or worse (b) computing a max-required-size that includes operations not guaranteed to execute on all paths, changing gas/behavior if precheck is emitted. Include OP_JUMPI as a terminator (and any other terminators your analyzer recognizes).
Suggested change
| return Opcode == OP_JUMP || Opcode == OP_RETURN || Opcode == OP_STOP || | |
| return Opcode == OP_JUMP || Opcode == OP_JUMPI || | |
| Opcode == OP_RETURN || Opcode == OP_STOP || |
Comment on lines
+853
to
+882
| case OP_PUSH9: | ||
| case OP_PUSH10: | ||
| case OP_PUSH11: | ||
| case OP_PUSH12: | ||
| case OP_PUSH13: | ||
| case OP_PUSH14: | ||
| case OP_PUSH15: | ||
| case OP_PUSH16: | ||
| case OP_PUSH17: | ||
| case OP_PUSH18: | ||
| case OP_PUSH19: | ||
| case OP_PUSH20: | ||
| case OP_PUSH21: | ||
| case OP_PUSH22: | ||
| case OP_PUSH23: | ||
| case OP_PUSH24: | ||
| case OP_PUSH25: | ||
| case OP_PUSH26: | ||
| case OP_PUSH27: | ||
| case OP_PUSH28: | ||
| case OP_PUSH29: | ||
| case OP_PUSH30: | ||
| case OP_PUSH31: | ||
| case OP_PUSH32: { | ||
| uint8_t NumBytes = | ||
| static_cast<uint8_t>(Opcode) - static_cast<uint8_t>(OP_PUSH0); | ||
| ScanPC += NumBytes; | ||
| SimStack.push_back(makeUnknownConstU64()); | ||
| break; | ||
| } |
There was a problem hiding this comment.
This path skips immediate bytes without checking ScanPC + NumBytes <= BytecodeSize. If the bytecode is malformed/truncated, the scan can incorrectly proceed and potentially mark a block eligible based on earlier state. For consistency with parsePushConstU64(...), add a bounds check and return {} when the immediate would exceed BytecodeSize.
Comment on lines
4282
to
+4286
| MInstruction *EVMMirBuilder::getMemoryDataPointer() { | ||
| ++MemStats.GetMemoryDataPointerCount; | ||
| if (CurBlockMemStats.Active) { | ||
| CurBlockMemStats.GetMemPtrCount++; | ||
| } |
There was a problem hiding this comment.
All stats counters and block tracking are updated unconditionally, even when ZEN_ENABLE_MULTIPASS_JIT_LOGGING is off (and dumpMemoryCompileStats() won’t emit anything). This adds overhead and increases EVMMirBuilder size in non-logging builds. Consider compiling stats fields + counter increments behind ZEN_ENABLE_MULTIPASS_JIT_LOGGING (or a dedicated instrumentation macro), or using a compile-time constant to fully optimize them out.
Comment on lines
690
to
+701
| void handleBeginBlock(EVMAnalyzer &Analyzer) { | ||
| const auto &BlockInfos = Analyzer.getBlockInfos(); | ||
| ZEN_ASSERT(BlockInfos.count(PC) > 0 && "Block info not found"); | ||
| Builder.beginMemoryCompileBlock(PC); | ||
| const Byte *Bytecode = Ctx->getBytecode(); | ||
| size_t BytecodeSize = Ctx->getBytecodeSize(); | ||
| BlockConstPrecheckPlan PrecheckPlan = | ||
| analyzeConstDirectMemoryBlockPrecheck(Bytecode, BytecodeSize, PC); | ||
| if (PrecheckPlan.Eligible) { | ||
| Builder.setMemoryCompileBlockConstPrecheckPlan( | ||
| PrecheckPlan.MaxRequiredSize, PrecheckPlan.CoveredDirectOps); | ||
| } |
There was a problem hiding this comment.
The PR title/description reads like it only adds an instrumentation summary, but this change introduces a behavior-changing optimization (block-local memory precheck planning + skipping per-op expansion checks). Please update the PR description/title and (if applicable per repo conventions) add a release note entry to reflect the optimization and its scope/constraints.
…ory ops Detect simple constant-address direct-memory blocks in the EVM bytecode visitor and install a block-local precheck plan before MIR generation. Reuse the existing expandMemoryIR path so covered MLOAD, MSTORE, and MSTORE8 ops share one precheck instead of emitting redundant per-op expansion CFG. Keep helper-sensitive, MCOPY, and dynamic-address blocks on the original path and retain development diagnostics behind the existing logging guard.
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.
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