Skip to content
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

DS-JS Backend #316

Merged
merged 49 commits into from
Dec 4, 2023
Merged

DS-JS Backend #316

merged 49 commits into from
Dec 4, 2023

Conversation

b-studios
Copy link
Collaborator

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.

@b-studios
Copy link
Collaborator Author

@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.

@b-studios
Copy link
Collaborator Author

b-studios commented Nov 27, 2023

@marzipankaiser could you help me debug the breakage in PolymorphismBoxing?
I added captures to BlockParams and guess the breakage is caused by the following change:

https://github.com/effekt-lang/effekt/pull/316/files#diff-e3c105bf7369dea9df7e42bc4aaa7ec444a2d5d472068ca01688ca38c5fd269cR343

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))),

@marzipankaiser
Copy link
Contributor

marzipankaiser commented Nov 28, 2023

Quick benchmark results of this(>) in comparison to old js results (<) that I had lying around.
Columns are Benchmark, mean [s], stddev.

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.

@b-studios
Copy link
Collaborator Author

@marzipankaiser I fixed a few things in Renamer and moved it from test/core to main/core since it is useful also to write transformations.

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.

@b-studios
Copy link
Collaborator Author

BTW the "Text file busy" CI bug is really getting annoying... #322

Comment on lines 154 to 176
// 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)
// }
Copy link
Collaborator Author

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.

@b-studios b-studios marked this pull request as ready for review December 4, 2023 16:28
@b-studios
Copy link
Collaborator Author

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?

@b-studios b-studios merged commit 9625f25 into master Dec 4, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants