-
Notifications
You must be signed in to change notification settings - Fork 31
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
Direct-style (DS) JS Backend #316
Conversation
@phischu observed that the generated continuations always are only used exactly once. We could inline them in the future. However, I am not sure how this could affect performance. |
@marzipankaiser could you help me debug the breakage in PolymorphismBoxing? Do we now somehow also adapt captures? What is the problem here? The part of the test that fails is (-expected, +got): - List(BlockParam(l3,Function(List(),List(),List(Var(A)),List(),Var(A)),Set(hhofargB))),
+ List(BlockParam(l3,Function(List(),List(),List(Var(A)),List(),Var(A)),Set(tmp58))), |
Quick benchmark results of this( 2c2
< countdown,171015.9,486.7
---
> countdown,33.9,0.7
5,10c5,10
< iterator,19537.8,64.2
< nqueens,34128.0,107.4
< product early,,
< resume nontail,,
< tree explore,,
< triples,3963.5,30.0
---
> iterator,33.2,0.3
> nqueens,34.1,0.3
> product early,39.6,0.3
> resume nontail,47.2,0.4
> tree explore,37.6,0.3
> triples,10473.5,38.1
Note: The results output by the above were wrong for, e.g. iterator (15 instead of 800000020000000) and resume_nontail (37 instead of 357). Am investigating, might be related to #318. |
e6ba9e5
to
1d113ea
Compare
@marzipankaiser I fixed a few things in Note that the implementation is still not correct. Local definitions are not mutually recursive (at the moment). Since renamer currently allows this, there might be some accidental capture. |
BTW the "Text file busy" CI bug is really getting annoying... #322 |
// test("shadowing let bindings"){ | ||
// val input = | ||
// """ module main | ||
// | | ||
// | def main = { () => | ||
// | let x = 1 | ||
// | let x = 2 | ||
// | return x:Int | ||
// | } | ||
// |""".stripMargin | ||
// | ||
// val expected = | ||
// """ module main | ||
// | | ||
// | def main = { () => | ||
// | let renamed1 = 1 | ||
// | let renamed2 = 2 | ||
// | return renamed2:Int | ||
// | } | ||
// |""".stripMargin | ||
// | ||
// assertRenamedTo(input, expected) | ||
// } |
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.
@marzipankaiser this test shows that Renamer
right now is not correct.
There are still few things to be done, like making sure that the monadic version still works and adapting the homepage to the new calling convention. However, I think this now starts to look good enough to become the new standard backend. I know it is a huge PR, but maybe someone is interested to look into it? |
This is the work-in-progress PR to add a direct-style JS backend. It builds on our work of bytecode instrumentation from 2018. Although I am implementing a variant here that is closer to generalized stack inspection.