Merged
Conversation
9ecf4e9 to
0eb5b0c
Compare
There was a problem hiding this comment.
Pull request overview
This PR simplifies the bytecode opcode space by eliminating dedicated immediate op variants and instead encoding immediates via a reserved register (u8::MAX / 0xFF), and removes the now-unneeded RegOpDiscriminants from fidget-core.
Changes:
- Remove
RegOpDiscriminantsgeneration/export fromfidget-core. - Revamp
fidget-bytecodeopcode representation to a smallerBytecodeOpset and update bytecode emission to use register0xFFas the “inline immediate” marker. - Update bytecode builder API to error if the reserved register is used, and add new dependencies (
serde,thiserror).
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| fidget-core/src/compiler/op.rs | Drops discriminant-deriving machinery from RegOp now that bytecode no longer relies on it. |
| fidget-core/src/compiler/mod.rs | Stops re-exporting RegOpDiscriminants. |
| fidget-bytecode/src/lib.rs | Introduces a simplified opcode enum and updates bytecode encoding to use reserved-register immediates; Bytecode::new now returns a Result. |
| fidget-bytecode/Cargo.toml | Adds serde + thiserror dependencies needed by the revised bytecode layer. |
| CHANGELOG.md | Documents the reserved-register bytecode revamp and removal of RegOpDiscriminants. |
| Cargo.lock | Locks newly introduced dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Remove dedicated immediate ops in favor of a reserved register. This experimentally improves GPU interpreter performance (not yet published).