Skip to content

minor improvements for pass pipeline #1424

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

Merged
merged 14 commits into from
Aug 5, 2020
Merged

minor improvements for pass pipeline #1424

merged 14 commits into from
Aug 5, 2020

Conversation

MaxGraey
Copy link
Member

@MaxGraey MaxGraey commented Aug 3, 2020

Add more pre-SSA passes like simplify-locals-notee-nostructure and move redundant set elimination before SSA.

  • I've read the contributing guidelines

@MaxGraey MaxGraey requested a review from dcodeIO August 3, 2020 06:50
@MaxGraey MaxGraey changed the title Slightly improve pass pipeline minor improvements for pass pipeline Aug 3, 2020
@dcodeIO
Copy link
Member

dcodeIO commented Aug 3, 2020

I worry a bit that we are adding so much passes to the pipeline (removing 4, adding 9, partly conditional) that optimization will become slower and slower while we keep doing this. I see that the additions reduce fixtures by about 330 lines, which is good in classical terms, and I'd lean towards merging, yet the relativistic concern remains that achieving the speed of light (zero lines of output) can only be achieved by supplying infinite energy (running passes forever) and is ultimately impossible because of diminishing returns unless becoming massless (optimizing no code at all).

Or: Where do we draw the line? How much slowdown are we willing to accept for what gain? And how can we measure this as a function of time taken vs code optimized?

@MaxGraey
Copy link
Member Author

MaxGraey commented Aug 3, 2020

Basically this attempts also speedup pipeline by doing some work earlier, before SSA and flattering. In my internal tests it's really little bit faster than previous scheme.

Where do we draw the line? How much slowdown are we willing to accept for what gain? And how can we measure this as a function of time taken vs code optimized?

It will be great have some compiler benchmark with aggregate perf metrics and ideally with CI integration

@MaxGraey
Copy link
Member Author

MaxGraey commented Aug 3, 2020

For npm run test:compiler -- --create (and via ts-node) roughly comparison:
master: Time 80896 ms
this pr: Time 77528 ms

Copy link
Member

@dcodeIO dcodeIO left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, LGTM :)

@dcodeIO
Copy link
Member

dcodeIO commented Aug 5, 2020

Since this touches a lot of fixtures, can you merge master into this PR and regenerate the fixtures? Mostly just in case, i.e. if an export not present before #1422 would now be present and somehow introduce a fixture-only conflict.

@MaxGraey
Copy link
Member Author

MaxGraey commented Aug 5, 2020

Sure, done! But it seems it hasn't any changes

@dcodeIO dcodeIO merged commit 176e932 into AssemblyScript:master Aug 5, 2020
@dcodeIO
Copy link
Member

dcodeIO commented Aug 5, 2020

Thanks!

@MaxGraey MaxGraey deleted the pipeline branch August 5, 2020 18:28
@github-actions
Copy link

github-actions bot commented Aug 6, 2020

🎉 This PR is included in version 0.14.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants