-
Notifications
You must be signed in to change notification settings - Fork 486
[Builtins] Allow casing on booleans #7029
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
8358a8f
to
5244c96
Compare
5244c96
to
8b55532
Compare
8b55532
to
2344b92
Compare
2344b92
to
c0df0c1
Compare
/benchmark nofib |
1 similar comment
/benchmark nofib |
/benchmark lists |
1 similar comment
/benchmark lists |
Click here to check the status of your benchmark. |
Comparing benchmark results of 'nofib' on '27d2bc3905' (base) and 'c0df0c158b' (PR) Results table
|
Click here to check the status of your benchmark. |
Comparing benchmark results of 'nofib' on '27d2bc3905' (base) and 'c0df0c158b' (PR) Results table
|
Click here to check the status of your benchmark. |
Comparing benchmark results of 'lists' on '27d2bc3905' (base) and 'c0df0c158b' (PR) Results table
|
Click here to check the status of your benchmark. |
Comparing benchmark results of 'lists' on '27d2bc3905' (base) and 'c0df0c158b' (PR) Results table
|
c0df0c1
to
06d1a16
Compare
…to effectfully/builtins/allow-casing-on-booleans
…to effectfully/builtins/allow-casing-on-booleans
. PIR.lamAbs () b (PLC.mkTyBuiltin @_ @Bool ()) | ||
. PIR.lamAbs () x (PLC.TyVar () a) | ||
. PIR.lamAbs () y (PLC.TyVar () a) | ||
$ PIR.kase () |
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.
@@ -211,6 +210,8 @@ program | |||
(\v -> List (Tuple2 PredKey (List v))) Rational -> ParamValue | |||
in | |||
let | |||
!ifThenElse : all a. bool -> a -> a -> a | |||
= /\a -> \(b : bool) (x : a) (y : a) -> case a b [y, x] |
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 checked, the corresponding UPLC doesn't mention ifThenElse
anywhere (that binding here is dead).
I also checked the unconditionally inlining ifThenElse
isn't a good idea: if it's not saturated we get worst performance and if it's saturated we get the same performance (because it's inlined in UPLC anyway).
let | ||
!x : integer = multiplyInteger n d' | ||
!y : integer = multiplyInteger n' d | ||
in |
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.
Weird that it stopped inlining. I checked, it's because of ifThenElse
not inlining. But still weird.
ExBudget {exBudgetCPU = ExCPU 856122341, exBudgetMemory = ExMemory 4215188} No newline at end of file | ||
ExBudget {exBudgetCPU = ExCPU 660323564, exBudgetMemory = ExMemory 3405515} |
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.
-22.9% CPU, -19.2% MEM.
ExBudget {exBudgetCPU = ExCPU 82853157, exBudgetMemory = ExMemory 359405} No newline at end of file | ||
ExBudget {exBudgetCPU = ExCPU 66780110, exBudgetMemory = ExMemory 307802} |
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.
-19.4% CPU, -14.4% MEM.
ExBudget {exBudgetCPU = ExCPU 540900171, exBudgetMemory = ExMemory 2593218} No newline at end of file | ||
ExBudget {exBudgetCPU = ExCPU 472454709, exBudgetMemory = ExMemory 2373180} |
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.
-12.7% CPU, -8.5% MEM.
force | ||
(force ifThenElse | ||
(case | ||
(equalsData obsScriptCred wdrlAtZero) |
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.
This is definitely safe to optimize, we know the exact type that equalsData
returns. But I'm not sure if it's worth doing it instead of the more general optimization pushing force
into any kind of case
having delay
in its branches.
Bool_match | ||
(ifThenElse | ||
{Bool} | ||
(case | ||
Bool | ||
(equalsData r_stake_cred (fstPair {data} {data} p)) | ||
True | ||
False) | ||
[False, True]) |
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.
Anyway, we should stop converting between the two representations of Bool
now that the built-in one should be about as efficient as SOP.
1960 No newline at end of file | ||
1798 |
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.
-8.3%
cpu: 441439 | ||
mem: 2102 | ||
size: 32 No newline at end of file | ||
cpu: 285390 | ||
mem: 1601 | ||
size: 24 |
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.
Nice.
…to effectfully/builtins/allow-casing-on-booleans
This adds support for casing on booleans and integers using
Case
, which is the first part of #6602.