-
Notifications
You must be signed in to change notification settings - Fork 500
Add CaseApply optimization
#7421
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
c8d872c to
47a0318
Compare
47a0318 to
b8810a2
Compare
budget changesplutus-benchmark/bitwise/test/9.6/8
plutus-benchmark/bitwise/test/9.6/Ed25519.golden.eval
plutus-benchmark/cardano-loans/test/9.6/main.golden.eval
plutus-benchmark/coop/test/9.6/authMpBurning.golden.eval
plutus-benchmark/coop/test/9.6/authMpMinting.golden.eval
plutus-benchmark/coop/test/9.6/certMpBurning.golden.eval
plutus-benchmark/coop/test/9.6/certMpMinting.golden.eval
plutus-benchmark/coop/test/9.6/fsMpBurning.golden.eval
plutus-benchmark/coop/test/9.6/fsMpMinting.golden.eval
plutus-benchmark/coop/test/9.6/mustBurnOwnSingleton.golden.eval
plutus-benchmark/linear-vesting/test/9.6/main.golden.eval
plutus-benchmark/lists/test/Lookup/9.6/match-builtin-list-10.golden.eval
plutus-benchmark/lists/test/Lookup/9.6/match-builtin-list-100.golden.eval
plutus-benchmark/lists/test/Lookup/9.6/match-builtin-list-5.golden.eval
plutus-benchmark/lists/test/Lookup/9.6/match-builtin-list-50.golden.eval
plutus-benchmark/lists/test/Lookup/9.6/match-scott-list-10.golden.eval
plutus-benchmark/lists/test/Lookup/9.6/match-scott-list-100.golden.eval
plutus-benchmark/lists/test/Lookup/9.6/match-scott-list-5.golden.eval
plutus-benchmark/lists/test/Lookup/9.6/match-scott-list-50.golden.eval
plutus-benchmark/lists/test/Sum/9.6/left-fold-built-in.golden.eval
plutus-benchmark/lists/test/Sum/9.6/left-fold-data.golden.eval
plutus-benchmark/lists/test/Sum/9.6/left-fold-scott.golden.eval
plutus-benchmark/lists/test/Sum/9.6/right-fold-built-in.golden.eval
plutus-benchmark/lists/test/Sum/9.6/right-fold-data.golden.eval
plutus-benchmark/lists/test/Sum/9.6/right-fold-scott.golden.eval
plutus-benchmark/nofib/test/9.6/clausify-F5.golden.eval
plutus-benchmark/nofib/test/9.6/knights10-4x4.golden.eval
plutus-benchmark/nofib/test/9.6/queens4-bt.golden.eval
plutus-benchmark/nofib/test/9.6/queens5-fc.golden.eval
plutus-benchmark/script-contexts/test/V1/9.6/checkScriptContext1-20.golden.eval
plutus-benchmark/script-contexts/test/V1/9.6/checkScriptContext1-4.golden.eval
plutus-benchmark/script-contexts/test/V1/9.6/checkScriptContext2-20.golden.eval
plutus-benchmark/script-contexts/test/V1/9.6/checkScriptContext2-4.golden.eval
plutus-benchmark/script-contexts/test/V2/9.6/checkScriptContext1-20.golden.eval
plutus-benchmark/script-contexts/test/V2/9.6/checkScriptContext1-4.golden.eval
plutus-benchmark/script-contexts/test/V2/9.6/checkScriptContext2-20.golden.eval
plutus-benchmark/script-contexts/test/V2/9.6/checkScriptContext2-4.golden.eval
plutus-benchmark/script-contexts/test/V2/9.6/sopFwdStakeTrick.golden.eval
plutus-benchmark/script-contexts/test/V3/9.6/checkScriptContext1-20.golden.eval
plutus-benchmark/script-contexts/test/V3/9.6/checkScriptContext1-4.golden.eval
plutus-benchmark/script-contexts/test/V3/9.6/checkScriptContext2-20.golden.eval
plutus-benchmark/script-contexts/test/V3/9.6/checkScriptContext2-4.golden.eval
plutus-benchmark/script-contexts/test/V3/Data/9.6/purposeIsWellFormed-4.golden.eval
plutus-ledger-api/test-plugin/Spec/Budget/9.6/geq1.golden.eval
plutus-ledger-api/test-plugin/Spec/Budget/9.6/geq2.golden.eval
plutus-ledger-api/test-plugin/Spec/Budget/9.6/geq3.golden.eval
plutus-ledger-api/test-plugin/Spec/Budget/9.6/geq4.golden.eval
plutus-ledger-api/test-plugin/Spec/Budget/9.6/geq5.golden.eval
plutus-ledger-api/test-plugin/Spec/Budget/9.6/gt1.golden.eval
plutus-ledger-api/test-plugin/Spec/Budget/9.6/gt2.golden.eval
plutus-ledger-api/test-plugin/Spec/Budget/9.6/gt3.golden.eval
plutus-ledger-api/test-plugin/Spec/Budget/9.6/gt4.golden.eval
plutus-ledger-api/test-plugin/Spec/Budget/9.6/gt5.golden.eval
plutus-ledger-api/test-plugin/Spec/Data/Budget/9.6/geq1.golden.eval
plutus-ledger-api/test-plugin/Spec/Data/Budget/9.6/geq2.golden.eval
plutus-ledger-api/test-plugin/Spec/Data/Budget/9.6/geq3.golden.eval
plutus-ledger-api/test-plugin/Spec/Data/Budget/9.6/geq4.golden.eval
plutus-ledger-api/test-plugin/Spec/Data/Budget/9.6/geq5.golden.eval
plutus-ledger-api/test-plugin/Spec/Data/Budget/9.6/gt1.golden.eval
plutus-ledger-api/test-plugin/Spec/Data/Budget/9.6/gt2.golden.eval
plutus-ledger-api/test-plugin/Spec/Data/Budget/9.6/gt3.golden.eval
plutus-ledger-api/test-plugin/Spec/Data/Budget/9.6/gt4.golden.eval
plutus-ledger-api/test-plugin/Spec/Data/Budget/9.6/gt5.golden.eval
plutus-ledger-api/test-plugin/Spec/Data/Budget/9.6/mintValueBurned.golden.eval
plutus-ledger-api/test-plugin/Spec/Data/Budget/9.6/mintValueMinted.golden.eval
plutus-tx-plugin/test/AsData/Budget/9.6/destructSum-manual.golden.eval
plutus-tx-plugin/test/AsData/Budget/9.6/destructSum.golden.eval
plutus-tx-plugin/test/AsData/Budget/9.6/recordFields.golden.eval
plutus-tx-plugin/test/Budget/9.6/constAccL.golden.eval
plutus-tx-plugin/test/Budget/9.6/constElL.golden.eval
plutus-tx-plugin/test/Budget/9.6/findIndexCheap.golden.eval
plutus-tx-plugin/test/Budget/9.6/findIndexEmptyList.golden.eval
plutus-tx-plugin/test/Budget/9.6/findIndexExpensive.golden.eval
plutus-tx-plugin/test/Budget/9.6/map1.golden.eval
plutus-tx-plugin/test/Budget/9.6/map2.golden.eval
plutus-tx-plugin/test/Budget/9.6/map3.golden.eval
plutus-tx-plugin/test/Budget/9.6/show.golden.eval
plutus-tx-plugin/test/Budget/9.6/sumL.golden.eval
plutus-tx-plugin/test/Budget/9.6/sumR.golden.eval
plutus-tx-plugin/test/BuiltinList/Budget/9.6/drop.golden.eval
plutus-tx-plugin/test/BuiltinList/Budget/9.6/take.golden.eval
plutus-tx-plugin/test/BuiltinList/Budget/9.6/uniqueElementJust.golden.eval
plutus-tx-plugin/test/CallTrace/9.6/successfullEvaluationYieldsNoTraceLog.golden.eval
|
| Constr ann idx (processNestedApp <$> ts) | ||
| processNestedApp (Case ann t bs) = | ||
| Case ann (processNestedApp t) (processNestedApp <$> bs) | ||
| processNestedApp t = t |
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.
Are you intentionally not using the transformOf termSubterms pattern we use in almost every other optimization? I really don't like catch-all clauses like that. We add a new AST node and we won't get any warning here, no indication that this should be updated.
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.
transformOf is bottom up, so It won't capture all nested applications. I agree with processNestedApp t = t. I'll make it so that there's no wildcard matc
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.
It says "bottom-up" but I think they mean something different, 'cause I wouldn't call that "bottom-up":
transformOf :: ASetter a b a b -> (b -> b) -> a -> b
transformOf l f = go where
go = f . over l goIf I'm wrong and transformOf can't be used here, you get a free pass on calling me an insult of your choice for a year.
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.
Nope. I'm right.
Consider,
(lam a [[[[a (con integer 1)] (con integer 2)] (con integer 3)] (con integer 4)])
If I use transformOf, it will do it from the inside like so it will yield
(lam
a-21
[
(case (constr 0 (con integer 1) (con integer 2) (con integer 3)) a-21)
(con integer 4)
]
)
whereas if I to recursion myself it will do the correct thing and yield
(lam
a-21
(case
(constr 0 (con integer 1) (con integer 2) (con integer 3) (con integer 4))
a-21
)
)
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.
I would never insult you <3
plutus-core/untyped-plutus-core/src/UntypedPlutusCore/Transform/CaseApply.hs
Outdated
Show resolved
Hide resolved
| (evalSimplifierT . caseReduce) | ||
| , T.test_scopingGood "case-apply" | ||
| (genTerm @DefaultFun) | ||
| T.BindingRemovalOk |
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.
I think it's BindingRemovalNotOk? Not sure. Does anything break if you make it that?
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.
also passes with BindingRemovalNotOkay. BindingRemovalNotOk should be correct.
|
I don't understand how this is an optimisation. Can you explain the logic behind it? |
|
You can check here: #7410 (comment) Basically, applying arguments using |
8d79313 to
253835b
Compare
| simplifyNTimes (_soMaxSimplifierIterations opts) >=> cseNTimes cseTimes | ||
| simplifyNTimes (_soMaxSimplifierIterations opts) | ||
| >=> cseNTimes cseTimes | ||
| >=> if _soCaseApply opts then caseApply else pure |
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.
its strange irregularity: flag is checked here for caseApply, but not checked for cse or simplifier above 🤷🏼♂️
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.
Other ones run in a loop while case apply only run once so I didn't wanna make a binding for stuff this simple
Adds "application through case/constr" optimization. Pretty decent free optimizations.
It is also possible to turn programs like
into
But this is only possible when the body of inner lambda doesn't reference variable bounded by all previous lambda abstractions. Maybe this is easier to do on PIR side.