Add EXP and SIGNEXTEND. #43
Conversation
Changed Files
|
Summary of ChangesHello @tcrypt25519, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the arithmetic capabilities of the system by adding a new Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces implementations for the EXP and SIGNEXTEND EVM opcodes. While the SIGNEXTEND implementation appears correct, a critical endianness mismatch has been identified in the jet_ops_exp builtin function. The EXP implementation incorrectly treats values as big-endian, despite the VM using host-endianness (little-endian) for stack and memory. This will lead to incorrect results for the EXP instruction on little-endian platforms and must be addressed. The overall structure of the changes is good, with one minor suggestion for readability.
There was a problem hiding this comment.
Pull request overview
Adds runtime/JIT plumbing to support the EVM EXP opcode by introducing a new runtime symbol and builtin, wiring it into the JIT symbol map, and updating the IR builder to call it. The PR also includes an implementation of SIGNEXTEND in the IR builder.
Changes:
- Add new runtime symbol
jet.ops.expand declare it in the runtime LLVM module. - Implement
jet_ops_expbuiltin and link it into the JIT symbol mapping. - Implement
SIGNEXTENDIR generation incrates/jet/src/builder/ops.rs.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/jet_runtime/src/symbols.rs | Adds FN_EXP symbol name for runtime lookup. |
| crates/jet_runtime/src/runtime_builder.rs | Declares external builtin jet.ops.exp in the runtime module. |
| crates/jet_runtime/src/builtins.rs | Adds jet_ops_exp runtime implementation. |
| crates/jet_runtime/Cargo.toml | Introduces ruint dependency for U256 math. |
| crates/jet/src/engine/mod.rs | Maps jet.ops.exp symbol to the Rust builtin in the JIT engine. |
| crates/jet/src/builder/ops.rs | Emits IR to call EXP builtin and implements SIGNEXTEND in IR. |
| crates/jet/src/builder/env.rs | Loads and exposes the new exp runtime function symbol. |
| Cargo.lock | Updates lockfile for the new dependency graph. |
The cache was hitting old entries built for static linking, causing "unable to find library -lLLVM-21" errors after adding the llvm21-1-prefer-dynamic feature to inkwell. Adding v2 prefix to force a fresh LLVM installation with dynamic libraries. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
The llvm-21-dev package only provides static libraries and headers. When inkwell uses llvm21-1-prefer-dynamic, the linker needs the shared library libLLVM-21.so, which comes from the libllvm21 package. This was causing intermittent "unable to find library -lLLVM-21" errors because the cached LLVM installations didn't include the shared library. Changes: - Add libllvm21 to apt-get install in scripts/install-llvm.sh - Bump CI cache key to v3 to force fresh installation with shared lib Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Track EVM memory length correctly by expanding memory before writes and updating memory_len to match EVM semantics (32-byte word-aligned). Implementation: - Add jet_mem_expand builtin that: * Checks for offset + size overflow * Rounds up to 32-byte boundaries * Updates memory_len monotonically * Reallocates and zeros new memory if needed - Hook MSTORE (32 bytes) and MSTORE8 (1 byte) to call expansion before write - Add memory_len field to TestContractRun for validation Tests (all passing): - MSTORE at offsets 0, 31, 32 expands to 32, 64, 64 bytes - MSTORE8 at offsets 0, 31, 32 expands to 32, 32, 64 bytes - Memory expansion is word-aligned and monotonic Closes #41 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* Add EXP. * style: apply rustfmt formatting 🤖 * fixes * ci: force LLVM cache invalidation for dynamic linking The cache was hitting old entries built for static linking, causing "unable to find library -lLLVM-21" errors after adding the llvm21-1-prefer-dynamic feature to inkwell. Adding v2 prefix to force a fresh LLVM installation with dynamic libraries. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix: install libllvm21 for dynamic linking support The llvm-21-dev package only provides static libraries and headers. When inkwell uses llvm21-1-prefer-dynamic, the linker needs the shared library libLLVM-21.so, which comes from the libllvm21 package. This was causing intermittent "unable to find library -lLLVM-21" errors because the cached LLVM installations didn't include the shared library. Changes: - Add libllvm21 to apt-get install in scripts/install-llvm.sh - Bump CI cache key to v3 to force fresh installation with shared lib Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * feat: implement memory expansion tracking for MSTORE and MSTORE8 Track EVM memory length correctly by expanding memory before writes and updating memory_len to match EVM semantics (32-byte word-aligned). Implementation: - Add jet_mem_expand builtin that: * Checks for offset + size overflow * Rounds up to 32-byte boundaries * Updates memory_len monotonically * Reallocates and zeros new memory if needed - Hook MSTORE (32 bytes) and MSTORE8 (1 byte) to call expansion before write - Add memory_len field to TestContractRun for validation Tests (all passing): - MSTORE at offsets 0, 31, 32 expands to 32, 64, 64 bytes - MSTORE8 at offsets 0, 31, 32 expands to 32, 32, 64 bytes - Memory expansion is word-aligned and monotonic Closes #41 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * ci: bump LLVM cache to v4 * fix: use div_ceil for memory expansion rounding * docs: Add process doc for adding opcodes. --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
No description provided.