Fix caching of matching on the version for `VerifyEd25519Signature`
Apparently matching on the version is only cached for the meaning of ConsByteString and not for the meaning of VerifyEd25519Signature. This is because for the former matching is done outside of the calls to makeBuiltinMeaning:
toBuiltinMeaning ver ConsByteString =
-- The costing function is the same for all versions of this builtin, but since the
-- denotation of the builtin accepts constants of different types ('Integer' vs 'Word8'),
-- the costing function needs to by polymorphic over the type of constant.
let costingFun :: ExMemoryUsage a => BuiltinCostModel -> a -> BS.ByteString -> ExBudget
costingFun = runCostingFunTwoArguments . paramConsByteString
-- See Note [Versioned builtins]
in case ver of
DefaultFunV1 -> makeBuiltinMeaning
@(Integer -> BS.ByteString -> BS.ByteString)
(\n xs -> BS.cons (fromIntegral @Integer n) xs)
costingFun
-- For versions other (i.e. larger) than V1, the first input must be in range [0..255].
-- See Note [How to add a built-in function: simple cases]
_ -> makeBuiltinMeaning
@(Word8 -> BS.ByteString -> BS.ByteString)
BS.cons
costingFun
and for the latter the matching is done in a let that gets inlined causing the matching appear inside of the first argument of makeBuiltinMeaning:
toBuiltinMeaning ver VerifyEd25519Signature =
let verifyEd25519Signature =
case ver of
DefaultFunV1 -> verifyEd25519Signature_V1
_ -> verifyEd25519Signature_V2
in makeBuiltinMeaning
verifyEd25519Signature
-- Benchmarks indicate that the two versions have very similar
-- execution times, so it's safe to use the same costing function for
-- both.
(runCostingFunThreeArguments . paramVerifyEd25519Signature)
The outcome is that VerifyEd25519Signature is a tiny bit less efficient than it could be. I don’t think we care about performance much in this case, but we should have a documented policy telling the user how to define versioned builtins in a way that allows caching of the matching on the version.
I can imagine two possible solutions:
-
as with
ConsByteStringjust have two calls tomakeBuiltinMeaning. But that’s kinda annoying and stupid, hence I’d prefer the next one (provided it works out) -
mark
verifyEd25519SignatureasNOINLINE(may not work if GHC annoyingly chooses to eta-expand the body of the case, which is fixable, but isn’t worth fixing) or, even better, uselet !verifyEd25519Signatureor, even betterer, a strict case-expression representing the same thing – any of that – in order to ensure that the matching is forced outside of the call tomakeBuiltinMeaning
Acceptance criteria: it’s clear in the Core that matching on the version of VerifyEd25519Signature is properly cached and there’s a documented policy on how versioned builtins need to be implemented.