-
Notifications
You must be signed in to change notification settings - Fork 501
Add --cse-which-subterms flag #7511
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
base: master
Are you sure you want to change the base?
Conversation
zliu41
left a comment
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.
Looks good, but we should look at how turning it on affects the costs and sizes in the existing tests. That will help decide whether or not it should be on by default.
|
I've taken a quick look at the five test cases that are there for CSE. The Output size on disk (in bytes) is slightly better in some cases, but equal in others: Size on disk is probably not a great measure, but I don't know of a simple way to measure AST size of the outputs. I'll check why the expensive test gets so much slower, perhaps it's just considering way more sub-expressions. |
|
The tests from |
|
Thanks for the data. I'm surprised that there are CPU improvements when allowing workfree terms. As I noted before, Do you know why we are seeing CPU improvements? If not, it's worth looking into the We should also update |
Also, does the |




This allows CSE to consider all sub-expressions. Currently it excludes work-free subexpressions such as partially applied built-ins (https://github.com/IntersectMBO/plutus-private/issues/1761).
factored out
isWorkFree'from the logic ofcse, makes it easier to understandinstead of
Bool, I've added aCseWhichSubtermstype to avoid boolean blindness. I found that it makes the code more understandable. If Bool is preferred for consistency with other simplifier flags, I'm happy to change it.