-
Notifications
You must be signed in to change notification settings - Fork 484
[Builtins] Add an inlinable version of 'geq' #7323
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?
Changes from all commits
b3caf59
8b9613f
31de858
eb83b4e
95242eb
d9866f6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -131,6 +131,57 @@ pattern DefaultUniArray uniA = | |
pattern DefaultUniPair uniA uniB = | ||
DefaultUniProtoPair `DefaultUniApply` uniA `DefaultUniApply` uniB | ||
|
||
-- Removing 'LoopBreaker' didn't change anything at the time this comment was written, but we kept | ||
-- it, because it hopefully provides some additional assurance that 'geqL' will not get elaborated | ||
-- as a recursive definition. | ||
instance AllBuiltinArgs DefaultUni (GEqL DefaultUni) a => GEqL DefaultUni a where | ||
geqL DefaultUniInteger a2 = do | ||
DefaultUniInteger <- pure a2 | ||
pure Refl | ||
geqL DefaultUniByteString a2 = do | ||
DefaultUniByteString <- pure a2 | ||
pure Refl | ||
geqL DefaultUniString a2 = do | ||
DefaultUniString <- pure a2 | ||
pure Refl | ||
geqL DefaultUniUnit a2 = do | ||
DefaultUniUnit <- pure a2 | ||
pure Refl | ||
geqL DefaultUniBool a2 = do | ||
DefaultUniBool <- pure a2 | ||
pure Refl | ||
geqL (DefaultUniProtoList `DefaultUniApply` a1) listA2 = do | ||
DefaultUniProtoList `DefaultUniApply` a2 <- pure listA2 | ||
Refl <- geqL (LoopBreaker a1) (LoopBreaker a2) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you not apply the same There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I probably should re-review this then. I thought by having |
||
pure Refl | ||
geqL (DefaultUniProtoArray `DefaultUniApply` a1) arrayA2 = do | ||
DefaultUniProtoArray `DefaultUniApply` a2 <- pure arrayA2 | ||
Refl <- geqL (LoopBreaker a1) (LoopBreaker a2) | ||
pure Refl | ||
geqL (DefaultUniProtoPair `DefaultUniApply` a1 `DefaultUniApply` b1) pairA2 = do | ||
DefaultUniProtoPair `DefaultUniApply` a2 `DefaultUniApply` b2 <- pure pairA2 | ||
Refl <- geqL (LoopBreaker a1) (LoopBreaker a2) | ||
Refl <- geqL (LoopBreaker b1) (LoopBreaker b2) | ||
pure Refl | ||
geqL (f `DefaultUniApply` _ `DefaultUniApply` _ `DefaultUniApply` _) _ = | ||
noMoreTypeFunctions f | ||
geqL DefaultUniData a2 = do | ||
DefaultUniData <- pure a2 | ||
pure Refl | ||
geqL DefaultUniBLS12_381_G1_Element a2 = do | ||
DefaultUniBLS12_381_G1_Element <- pure a2 | ||
pure Refl | ||
geqL DefaultUniBLS12_381_G2_Element a2 = do | ||
DefaultUniBLS12_381_G2_Element <- pure a2 | ||
pure Refl | ||
geqL DefaultUniBLS12_381_MlResult a2 = do | ||
DefaultUniBLS12_381_MlResult <- pure a2 | ||
pure Refl | ||
geqL DefaultUniValue a2 = do | ||
DefaultUniValue <- pure a2 | ||
pure Refl | ||
{-# INLINE geqL #-} | ||
|
||
instance GEq DefaultUni where | ||
-- We define 'geq' manually instead of using 'deriveGEq', because the latter creates a single | ||
-- recursive definition and we want two instead. The reason why we want two is because this | ||
|
@@ -140,59 +191,63 @@ instance GEq DefaultUni where | |
-- (we're not really sure if this is a reliable solution, but if it stops working, we won't miss | ||
-- very much and we've failed to settle on any other approach). | ||
-- | ||
-- This trick gives us a 1% speedup across validation benchmarks (some are up to 4% faster) and | ||
-- a more sensible generated Core where things like @geq DefaulUniBool@ are reduced away. | ||
geq = geqStep where | ||
geqStep :: DefaultUni a1 -> DefaultUni a2 -> Maybe (a1 :~: a2) | ||
geqStep DefaultUniInteger a2 = do | ||
DefaultUniInteger <- Just a2 | ||
Just Refl | ||
geqStep DefaultUniByteString a2 = do | ||
DefaultUniByteString <- Just a2 | ||
Just Refl | ||
geqStep DefaultUniString a2 = do | ||
DefaultUniString <- Just a2 | ||
Just Refl | ||
geqStep DefaultUniUnit a2 = do | ||
DefaultUniUnit <- Just a2 | ||
Just Refl | ||
geqStep DefaultUniBool a2 = do | ||
DefaultUniBool <- Just a2 | ||
Just Refl | ||
geqStep DefaultUniProtoList a2 = do | ||
DefaultUniProtoList <- Just a2 | ||
Just Refl | ||
geqStep DefaultUniProtoArray a2 = do | ||
DefaultUniProtoArray <- Just a2 | ||
Just Refl | ||
geqStep DefaultUniProtoPair a2 = do | ||
DefaultUniProtoPair <- Just a2 | ||
Just Refl | ||
geqStep (DefaultUniApply f1 x1) a2 = do | ||
DefaultUniApply f2 x2 <- Just a2 | ||
Refl <- geqRec f1 f2 | ||
Refl <- geqRec x1 x2 | ||
Just Refl | ||
geqStep DefaultUniData a2 = do | ||
DefaultUniData <- Just a2 | ||
Just Refl | ||
geqStep DefaultUniBLS12_381_G1_Element a2 = do | ||
DefaultUniBLS12_381_G1_Element <- Just a2 | ||
Just Refl | ||
geqStep DefaultUniBLS12_381_G2_Element a2 = do | ||
DefaultUniBLS12_381_G2_Element <- Just a2 | ||
Just Refl | ||
geqStep DefaultUniBLS12_381_MlResult a2 = do | ||
DefaultUniBLS12_381_MlResult <- Just a2 | ||
Just Refl | ||
geqStep DefaultUniValue a2 = do | ||
DefaultUniValue <- Just a2 | ||
Just Refl | ||
{-# INLINE geqStep #-} | ||
|
||
geqRec :: DefaultUni a1 -> DefaultUni a2 -> Maybe (a1 :~: a2) | ||
geqRec = geqStep | ||
{-# OPAQUE geqRec #-} | ||
-- On the critical path this definition should only be used for builtins that perform equality | ||
-- checking of statically unknown runtime type tags ('MkCons' is one such builtin for | ||
-- example). All other builtins should use 'geqL' (the latter is internal to 'readKnownConstant' | ||
-- and is therefore hidden from the person adding a new builtin). | ||
-- | ||
-- We use @NOINLINE@ instead of @OPAQUE@, because we don't actually care about the recursive | ||
-- definition not being inlined, we just want it to be chosen as the loop breaker. | ||
geq = goStep where | ||
goStep, goRec :: DefaultUni a1 -> DefaultUni a2 -> Maybe (a1 :~: a2) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there actual meaningful change here, or just renaming? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just renaming, I was playing with other stuff and decided that there should be a convention to call such functions |
||
goStep DefaultUniInteger a2 = do | ||
DefaultUniInteger <- pure a2 | ||
pure Refl | ||
goStep DefaultUniByteString a2 = do | ||
DefaultUniByteString <- pure a2 | ||
pure Refl | ||
goStep DefaultUniString a2 = do | ||
DefaultUniString <- pure a2 | ||
pure Refl | ||
goStep DefaultUniUnit a2 = do | ||
DefaultUniUnit <- pure a2 | ||
pure Refl | ||
goStep DefaultUniBool a2 = do | ||
DefaultUniBool <- pure a2 | ||
pure Refl | ||
goStep DefaultUniProtoList a2 = do | ||
DefaultUniProtoList <- pure a2 | ||
pure Refl | ||
goStep DefaultUniProtoArray a2 = do | ||
DefaultUniProtoArray <- pure a2 | ||
pure Refl | ||
goStep DefaultUniProtoPair a2 = do | ||
DefaultUniProtoPair <- pure a2 | ||
pure Refl | ||
goStep (DefaultUniApply f1 x1) a2 = do | ||
DefaultUniApply f2 x2 <- pure a2 | ||
Refl <- oneShot goRec f1 f2 | ||
Refl <- oneShot goRec x1 x2 | ||
pure Refl | ||
goStep DefaultUniData a2 = do | ||
DefaultUniData <- pure a2 | ||
pure Refl | ||
goStep DefaultUniBLS12_381_G1_Element a2 = do | ||
DefaultUniBLS12_381_G1_Element <- pure a2 | ||
pure Refl | ||
goStep DefaultUniBLS12_381_G2_Element a2 = do | ||
DefaultUniBLS12_381_G2_Element <- pure a2 | ||
pure Refl | ||
goStep DefaultUniBLS12_381_MlResult a2 = do | ||
DefaultUniBLS12_381_MlResult <- pure a2 | ||
pure Refl | ||
goStep DefaultUniValue a2 = do | ||
DefaultUniValue <- pure a2 | ||
pure Refl | ||
{-# INLINE goStep #-} | ||
|
||
goRec = goStep | ||
{-# NOINLINE goRec #-} | ||
|
||
-- | For pleasing the coverage checker. | ||
noMoreTypeFunctions :: DefaultUni (Esc (f :: a -> b -> c -> d)) -> any | ||
|
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.
What does the superclass constraint,
AllBuiltinArgs...
, do? It doesn't seem to provide any method needed to implementgeqL
?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's there so that the recursive call instead of being elaborated as a recursive call becomes a lookup in the
AllBuiltinArgs
dictionary. You can seeGEqL
being the constraint thatAllBuiltinArgs
is applied to. Now instead of having term-level recursion, you have type-level recursion -- and that one is inlining-friendly.