-
Notifications
You must be signed in to change notification settings - Fork 323
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
Removal of useless ApplicationSaturation phase #8181
Removal of useless ApplicationSaturation phase #8181
Conversation
Benchmark runs:
|
ApplicationSaturation -->> ApplicationSaturation.Configuration(), | ||
AliasAnalysis -->> AliasAnalysis.Configuration() |
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.
Was the ApplicationSaturation
pass ever actually used outside of tests??
Looking at this initializer, we can see that the Configuration
gets initialized with an empty Map
for knownFunctions
. And if the encountered funtion is not one of knownFunctions
, it seems to always get the Unknown
state, not influencing the codegen in any way.
I also don't see any other place where this pass is configured to "know" any functions and be able to do anything useful.
So I suspect we shouldn't see any difference in benchmarks, because we seem to be removing already practically unused code.
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.
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.
Unused compiler pass? I had no idea we even had ApplicationSaturation
. Good observation.
Benchmarks seem to be unaffected. Merging. |
A note by @kustosz:
|
Pull Request Description
Work on #8172 has revealed that
ApplicationSaturation
phase of the IR compiler is kind of special. It pre-executes saturated applications help the compiler replace code with constants. While such IR compiler pass may be needed in a static compilation, it is very likely useless in dynamicaly compiled language like Enso. Let's verify it by removing the pass and checking effect of such change on benchmarks.Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,