Implement IR-based runtime with unified memory model and generated builtins#19
Conversation
Changed Files
|
…emory layout Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>
…ndwritten LLVM Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>
Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>
…ing comments Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>
…ntation summary Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>
tcrypt25519
left a comment
There was a problem hiding this comment.
I mentioned a few times throughout my comments there being 80+ files that were changed, which is what it said when I started writing the comments. I see it's now down to 18, which is a much more reasonable number. So I don't want to go through and update everywhere I said that, but I do want to say here that whatever exactly changed from when I started leaving these comments until now seems to have been a positive direction.
… decision Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>
Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>
- Generate all stack operations (push, pop, peek, swap) as LLVM IR - Generate all memory operations (load, store word, store byte) as LLVM IR - Keep contract calls and crypto operations as external Rust functions - Remove Rust implementations of stack/memory ops from builtins.rs - Update engine to only link external functions, not IR-generated ones - This fulfills the core requirement of ADR-001 Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR implements ADR-001 and resolves critical memory layout mismatches by creating an IR-based runtime architecture with a unified memory model. The changes introduce a new jet_ir crate containing shared LLVM type definitions, implement a RuntimeBuilder that programmatically generates 8 runtime builtin functions as LLVM IR, and migrate the execution context to a pointer-based memory representation.
Changes:
- Created new
jet_ircrate with unifiedTypesstruct and shared constants - Implemented
RuntimeBuilderto generate 8 runtime functions as IR (stack operations: push.i256, push.word, pop, peek, swap; memory operations: load, store.word, store.byte) - Migrated
Contextstruct from inline array to pointer-based memory layout with heap allocation and Drop implementation - Fixed ADDRESS_SIZE_BYTES from 2 to 20 bytes (EVM standard)
- Removed handwritten runtime functions from builtins.rs (now generated as IR)
- Updated engine to use RuntimeBuilder instead of loading runtime-ir/jet.ll
- Created ADR-002 documenting pointer-based memory decision
- Added layout verification tests
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
Cargo.toml |
Added jet_ir to workspace members |
crates/jet_ir/* |
New crate with unified Types and constants |
crates/jet_runtime/src/runtime_builder.rs |
New RuntimeBuilder generating 8 IR functions |
crates/jet_runtime/src/exec.rs |
Pointer-based memory with heap allocation and Drop |
crates/jet_runtime/src/builtins.rs |
Removed stack/memory functions (now IR-generated) |
crates/jet_runtime/src/lib.rs |
Re-exports from jet_ir for backward compatibility |
crates/jet_runtime/src/layout_tests.rs |
New layout verification tests |
crates/jet_runtime/Cargo.toml |
Added jet_ir dependency |
crates/jet/src/engine/mod.rs |
Uses RuntimeBuilder instead of loading jet.ll |
crates/jet/src/builder/env.rs |
Uses shared Types from jet_ir |
crates/jet/src/bin/jetdbg.rs |
Fixed coinbase address to 20 bytes |
crates/jet/Cargo.toml |
Added jet_ir dependency |
docs/adrs/adr-002.md |
Documents pointer-based memory decision |
docs/architecture-refactoring-summary.md |
Comprehensive refactoring documentation |
runtime-ir/DEPRECATED.md |
Deprecation notice for jet.ll |
IMPLEMENTATION_COMPLETE.md |
Implementation completion summary |
|
@copilot Fix these issues: Fix function name mismatch for stack push File: crates/jet_runtime/src/runtime_builder.rs Comment: The IR function is created with the name jet.stack.push.word, but the symbol constant FN_STACK_PUSH_PTR expects jet.stack.push.ptr. This mismatch will cause a runtime lookup failure. Suggested change from reviewer: Change the function name passed to add_function to match the symbol Correct runtime function count in IMPLEMENTATION_COMPLETE.md File: IMPLEMENTATION_COMPLETE.md Comment: The documentation claims only jet.stack.push.i256 is generated as IR, but the implementation actually generates eight IR runtime functions. Suggested change from reviewer: Replace the statement about a single IR function with an accurate list:
Clarify that remaining runtime behavior still uses Rust builtins and that Update “Future Work” section to reflect current IR coverage File: docs/architecture-refactoring-summary.md Comment: The “Future Work” section says only jet.stack.push.i256 exists as IR, but eight IR runtime functions are already implemented. Suggested change from reviewer: Update the bullet to list the currently implemented IR runtime functions:
|
- Fix jet.stack.push.word -> jet.stack.push.ptr to match symbol constant - Update IMPLEMENTATION_COMPLETE.md to list all 8 IR-generated functions - Update architecture-refactoring-summary.md to list all 8 IR-generated functions Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>
... Fixed all three issues in commit 436ad86:
The documentation now accurately reflects that 8 runtime functions are generated as IR, not just one. |
tcrypt25519
left a comment
There was a problem hiding this comment.
@copilot Fix the rest of your dogshit code. I left you comments on the ones that I saw, but there's probably more, so please, after you fix the things that I called out, look over everything you did, realize how much of it sucks, and fix anything that you are cognitively able to do.
- Remove IMPLEMENTATION_COMPLETE.md (doesn't belong in repo) - Remove runtime-ir/DEPRECATED.md and runtime-ir/jet.ll (no longer used) - Rewrite architecture-refactoring-summary.md to describe current state not transition - Remove "backward compatibility" comment from lib.rs - Change comment in exec.rs from "Changed to" to simply state current state Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>
Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>
Fixed all issues in commits f275036 and 1db91e8:
|
…iltins (#19) * Initial plan * Phase 1 complete: Create jet_ir crate with unified Types and update memory layout Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> * Phase 2 complete: Create runtime builder to generate IR instead of handwritten LLVM Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> * Add documentation and layout verification tests Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> * Address code review feedback: improve memory handling and add clarifying comments Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> * Final refinement: simplify return_data slice creation and add implementation summary Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> * Remove excessive comments and create ADR-002 for pointer-based memory decision Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> * Refactor layout tests to use enums instead of magic numbers Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> * Implement runtime builtins as generated LLVM IR instead of compiled Rust - Generate all stack operations (push, pop, peek, swap) as LLVM IR - Generate all memory operations (load, store word, store byte) as LLVM IR - Keep contract calls and crypto operations as external Rust functions - Remove Rust implementations of stack/memory ops from builtins.rs - Update engine to only link external functions, not IR-generated ones - This fulfills the core requirement of ADR-001 Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> * Fix function name mismatch and update documentation - Fix jet.stack.push.word -> jet.stack.push.ptr to match symbol constant - Update IMPLEMENTATION_COMPLETE.md to list all 8 IR-generated functions - Update architecture-refactoring-summary.md to list all 8 IR-generated functions Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> * Remove transition documentation and unnecessary comments - Remove IMPLEMENTATION_COMPLETE.md (doesn't belong in repo) - Remove runtime-ir/DEPRECATED.md and runtime-ir/jet.ll (no longer used) - Rewrite architecture-refactoring-summary.md to describe current state not transition - Remove "backward compatibility" comment from lib.rs - Change comment in exec.rs from "Changed to" to simply state current state Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> * Clean up redundant project name mentions in documentation Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>
IR-based runtime with unified memory model and generated builtins
Problem
Memory layout defined inconsistently across three locations caused struct size mismatches (33KB vs 65KB) and field offset corruption. Handwritten LLVM IR was brittle and diverged from Rust
Contextstruct. Runtime builtin functions needed to be generated as LLVM IR (per ADR-001) instead of being compiled from Rust.Changes
New
jet_ircrate:Types<'ctx>struct containing all LLVM typesRuntimeBuilder - Full IR Generation:
push.i256,push.ptr,pop,peek,swapload,store.word,store.byteUnified memory model:
{ptr, len, cap}representation (ADR-002)Contextheap-allocates memory bufferTypesfromjet_irDocumentation:
Impact
TypesImplementation Details
IR-Generated Functions (8):
jet.stack.push.i256- Push i256 value onto stackjet.stack.push.ptr- Push word by pointer onto stackjet.stack.pop- Pop word from stackjet.stack.peek- Peek at stack elementjet.stack.swap- Swap stack elementsjet.mem.load- Load word from memoryjet.mem.store.word- Store word to memoryjet.mem.store.byte- Store byte to memoryExternal Rust Functions (3):
Notes
ADDRESS_SIZE_BYTES: 20 bytes (EVM standard)Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.