-
-
Notifications
You must be signed in to change notification settings - Fork 404
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Instruction
and InstructionIterator
#3201
Conversation
Test262 conformance changes
|
Codecov Report
@@ Coverage Diff @@
## main #3201 +/- ##
==========================================
- Coverage 50.42% 50.34% -0.09%
==========================================
Files 436 436
Lines 42549 42618 +69
==========================================
Hits 21457 21457
- Misses 21092 21161 +69
|
b952db4
to
776a25b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice improvement! It simplifies our logic a LOT.
Any plans for extending it to Operation
implementers or is it just not worth?
776a25b
to
9922448
Compare
No, this representation is intended for easier bytecode analysis, not really execution, the compact bytecode version is smaller and faster. |
} | ||
} | ||
|
||
macro_rules! generate_opcodes { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we start writing implementation notes to break down macros?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good to me. Nice work! Just one thought I had while reviewing the macro.
9922448
to
b2a8126
Compare
b2a8126
to
cfe700b
Compare
Whats the affect on performance with this change? |
There is no change in performance, this is doesn't change the way we execute bytecode. It only implements an iterator and the result being an |
This PR adds a fat enum that represents a full instruction (opcode + operands) and an iterator that takes bytecode and produces instructions.
I implemented something like this in #3037, this PR just extracts and refines it.
Why is this useful/needed?
instruction_operands()
and flowgraph generation, this is duplicate code and can easily lead to mistakes (also doing it in Implement control flow graph construction #3037 )Note: This is not intended to replace the compact bytecode representation, that is optimized for execution and storage.
... This also makes it easier to implement an bytecode size optimization on operands, that I'm working on ;)