Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes the JIT lowering of the EVM RETURNDATASIZE opcode by avoiding a runtime helper call and instead loading a cached return-data size directly from zen::runtime::EVMInstance.
Changes:
- Add a cached
ReturnDataSizefield toEVMInstance, keep it updated onsetReturnData()/clearReturnData(), and expose it viagetReturnDataSize()plus an offset getter for JIT. - Update
resetForNewCall()to clear return data viaclearReturnData()(also resetting the cached size). - Change MIR generation for
RETURNDATASIZEto inline-load the size from the instance field; update the imported helper to return the cached size.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/runtime/evm_instance.h | Introduces cached ReturnDataSize, accessors, and a JIT offset getter. |
| src/runtime/evm_instance.cpp | Resets return data via clearReturnData() during call reset. |
| src/compiler/evm_frontend/evm_mir_compiler.cpp | Lowers RETURNDATASIZE as a direct instance-field load instead of a runtime call. |
| src/compiler/evm_frontend/evm_imported.cpp | Makes evmGetReturnDataSize() return the cached size. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -86,7 +86,7 @@ void EVMInstance::resetForNewCall(evmc_revision NewRev) { | |||
| MemoryStack.clear(); | |||
|
|
|||
| // Reset output | |||
There was a problem hiding this comment.
In resetForNewCall(), switching from ReturnData.clear() to clearReturnData() changes the buffer-retention behavior: clearReturnDataBuffer() will deallocate when capacity > ReturnDataReleaseThreshold (64KB). If callers frequently return large data, this can introduce repeated alloc/free churn across calls. If this is intentional, consider adding a short comment here documenting the memory/perf tradeoff; otherwise consider preserving the previous behavior (clear size without releasing capacity) while still resetting ReturnDataSize.
Suggested change
| // Reset output | |
| // Reset output. | |
| // Note: clearReturnData() may release the underlying buffer when its | |
| // capacity exceeds ReturnDataReleaseThreshold to cap peak memory usage, | |
| // at the cost of possible alloc/free churn if large return data is common. |
⚡ 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 |
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