Inlining of (not all) control flow structures #39
Conversation
This file contains 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
…ined blocks, not sure how to fix that yet
…d unused. I wish they were usable, but afaik it's not doable in Rust to replace matches with more clever static array lookups (at least not with enums that have optional arguments)
… though: "self" in inlined blocks no longer points to the right value, so Bounce fails.
… (Mandelbrot infinite loop)
…e (had to make some compiler.rs data structures public)
…d moving some more logic to backpatch_jump() )
|
Just added Thanks for fixing the benchmark runner. Also w.r.t the specialized bytecode branch, here it is: #40.
Oops, my bad. Will fix that one.
Ah, that does make sense. I thought it was a bug, but it's nice that it doesn't seem to be. |
|
Here are the benchmark results for AST interpreter
Bytecode interpreter
|
|
Patches to the SOM benchmark harness or specific benchmarks to give more useful feedback are always welcome :) And indeed, some benchmarks don't work with all possible parameters. And yeah, zero iterations should probably also produce a warning to the user. Since it's likely an intended error :) (Especially since quite a while ago (multiple years, I believe) I changed the harness to not support warmup anymore, since I think it should be handled outside the harness and supporting it leads to unexpected recompilation by a JIT. Removing it also meant removing the middle |
|
Yeah, thinking about it, I've no idea why I was telling it to do 0 inner iterations. I'll consider submitting some changes to the benchmark harness since those errors confounded me for longer than I care to admit |
Hirevo
added
M-interpreter
Merged
…x_pop etc logic for jumps, breaks in some cases
|
Here are the benchmark results for AST interpreter
Bytecode interpreter
|
|
Feel free to review that one as is. It's got a lot of stuff to improve upon, but I'd appreciate initial feedback since there are currently a number of design decisions I'm unsure about, and which I'm positive you'll identify and possibly find flaws in. Sent you an email regarding the AST interpreter, btw! |
…e need for ast_body in Block
|
Here are the benchmark results for AST interpreter
Bytecode interpreter
|
…lining returnnonlocal
…rf is inaffected, then this is fine
|
Here are the benchmark results for AST interpreter
Bytecode interpreter
|
…e comment as to why
|
Here are the benchmark results for AST interpreter
Bytecode interpreter
|
|
Here are the benchmark results for AST interpreter
Bytecode interpreter
|
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.
Calls to:
ifTrue:ifFalse:ifTrue:ifFalseifFalse:ifTruewhileTrue:whileFalse:...are no longer treated as method calls and their instructions are inlined into the previous scope. This relies on several
JumpXbytecodes which modify the bytecode index to go back/further in the body of the method being executed.Some inlined control flow structures are missing, e.g.
to:do,and:, etc. But I'm optimistic they're easy to implement since the logic they rely on should already be present in this PR.I'm not sure what the speedup is as of yet, but it can be pushed further than it is right now since it has a few shortcomings and design decisions I'm unsure of: those are usually marked by a
TODOcomment in my code. I especially dislike the change I had to do tosom-interpreter-bc/src/block.rs. In short, it's almost ready for merging, and since it's functional on my machine (I may have missed stuff though!) I figured it was time to at least open the PR, to show that these changes do exist.I had started this work a year ago for fun and then dropped it. I picked it up recently since I was considering using
som-rsfor some research.Also now that I'm typing all this out, I realize this PR also contains some specialized bytecode like(EDIT: I've removed those from this branch now) Additionally, I implemented thePushConstant[0|1|2]orSend[1|2|3]which I also implemented a year ago...halt:primitive and removed the bytecode for it. Let me know if you want those changes to be proposed in their own PR.Let me know what you think. I don't know when I'll do the finishing touches for it, but hopefully soon.