[prototype] Support `base`'s classes and methods in Plutus Tx #5219

zliu41 · 2023-03-20T03:45:22Z

This PR requires a patched GHC, so it won't be mergeable for a while. But it is reviewable and I'm looking to get some early feedback.

This PR adds support for Haskell's Eq, Ord and Num. Other classes can be supported in a similar way. The benefits of doing so include:

One would be able to write regular Haskell rather than studying and using PlutusTx's version of base, and having to make sure things are imported from the right places.
It opens up the possibilities of using third party libraries.
Things like matching on integer literals and list range syntax will work (the latter doesn't currently work because we'd need to support enumFromTo, but is easily fixable)
This approach, with the help of the GHC patch, makes it easier to generate good error messages for unsupported features, e.g., we will be able to report Unsupported: (==) @Int rather than Unsupported: I#.

User migration will be easy because one can freely mix Haskell's and PlutusTx's typeclasses.

I've updated the modules in plutus-benchmark to use Haskell's typeclasses whenever possible. There are some cost increases, and as far as I can tell, they are all due to the fact that Haskell's Eq has two methods. This can be fixed separately in multiple ways, e.g., with a better inliner, or with some tricks in the plugin itself.

The main changes are in PlutusTx.Compiler.Expr and PlutusTx.Compiler.Dictionary.*. Other changes are mostly boilerplate.

The GHC patch is a fairly simple one: it adds a field in SimplMode and a GHC flag to turn off builtin rules, which stops GHC from turning (==) @Integer into $fEqInteger_$c==.

bezirg · 2023-03-21T09:00:55Z

Great work, is this GHC patch going to be mainlined? If so is there a MR to track it?

bezirg · 2023-03-21T09:01:16Z

.stylish-haskell.yaml

@@ -11,7 +11,7 @@ steps:
      remove_redundant: false

  - trailing_whitespace: {}
-columns: 100
+columns: 99


bezirg · 2023-03-21T09:04:14Z

cabal.project

@@ -32,6 +32,7 @@ packages: doc/read-the-docs-site
          prettyprinter-configurable
          word-array
          stubs/plutus-ghc-stub
+with-compiler: /home/zliu41/ghc/_build/stage1/bin/ghc


Maybe we can integrate the GHC patch with Nix inside a local .nix file?

bezirg · 2023-03-21T09:11:34Z

plutus-ledger-api/src/PlutusLedgerApi/V1/Contexts.hs

+instance Haskell.Eq ScriptContext where
+    {-# INLINABLE (==) #-}
+    (==) = (==)


suggestion: use some module qualification to make it look less like a loop

Also, what is the reasoning behind this explicit instance instead of the derived one? I am not saying it is wrong, but since we are transitioning to Haskell's builtin Eq,Ord,etc then why not use its builtin deriving as well?

bezirg · 2023-03-21T09:33:32Z

plutus-tx-plugin/src/PlutusTx/Plugin.hs

@@ -161,6 +161,7 @@ mkSimplPass flags logger =
            , GHC.sm_uf_opts = GHC.defaultUnfoldingOpts
            , GHC.sm_dflags = flags
            , GHC.sm_rules = False
+            , GHC.sm_builtin_rules = False


Nice! Add a comment here or a note on why we have this set to False (default for ghc being True)

bezirg · 2023-03-21T10:15:08Z

plutus-tx-plugin/src/PlutusTx/Compiler/Types.hs

@@ -174,13 +177,69 @@ stableModuleCmp m1 m2 =
    -- See Note [Stable name comparisons]
    (GHC.moduleUnit m1 `GHC.stableUnitCmp` GHC.moduleUnit m2)

+class Monad m => MonadCoreM m where


You forgot thelookupThing method.
Wouldn't it be better to just add some orphan instances of the MonadThings class instead? Then only the thNameToGhcName method would be missing, but then you could create a subclass:

class MonadThings m => MonadCoreM m where thNameToGhcName = ...

https://hackage.haskell.org/package/ghc-9.2.7/docs/GHC-Types-TyThing.html#t:MonadThings

michaelpj

The implementation seems very fiddly at the moment, I'd be keen to try and reduce that. I also think we probably shouldn't actually merge this unless we can optimize away the worse codegen for basic equality and comparison operations. But it's relatively clear how to do that at least in the monomorphic case, which is most important.

michaelpj · 2023-03-20T11:34:16Z

plutus-tx-plugin/src/PlutusTx/Compiler/Expr.hs

@@ -706,8 +715,6 @@ compileExpr e = withContextM 2 (sdToTxt $ "Compiling expr:" GHC.<+> GHC.ppr e) $
        GHC.Var (isErrorId -> True) `GHC.App` GHC.Type t `GHC.App` _ ->
            PIR.TyInst annMayInline <$> errorFunc <*> compileTypeNorm t

-        -- See Note [Uses of Eq]
-        GHC.Var n | GHC.getName n == GHC.eqName -> throwPlain $ UnsupportedError "Use of == from the Haskell Eq typeclass"
        GHC.Var n | GHC.getName n == GHC.integerEqName -> throwPlain $ UnsupportedError "Use of Haskell Integer equality, possibly via the Haskell Eq typeclass"


I guess this is still bad? i.e. if you get the non-typeclass integer equality function, we still want to die?

michaelpj · 2023-03-20T11:34:52Z

plutus-tx-plugin/src/PlutusTx/Compiler/Expr.hs

+    Maybe (m (PIRTerm uni fun))
+isKnownApp fun args = Map.lookup (splitGhcName (GHC.varName fun)) knownApps >>= ($ args)
+
+-- | A list of applications that should be compiled in specific ways.


This is big and complicated, let's put it in its own module.

michaelpj · 2023-03-20T11:37:02Z

plutus-tx-plugin/src/PlutusTx/Compiler/Expr.hs

+-- | A list of applications that should be compiled in specific ways.
+knownApps ::
+    CompilingDefault uni fun m ann =>
+    Map (Maybe String, String) ([GHC.CoreExpr] -> Maybe (m (PIRTerm uni fun)))


Why not just define this as a function?

mkKnownApp :: Maybe String -> String -> [GHC.CoreExpr] -> Maybe (m (PIRTerm uni fun))

I think that would make it more readable, and generally I think if you have a statically known map that's only used to do lookups it should probably just be a function?

Haven't reviewed this in full, I think it'll be a lot more readable as a function 😇

michaelpj · 2023-03-20T11:40:18Z

plutus-tx-plugin/src/PlutusTx/Compiler/Expr.hs

+    Map (Maybe String, String) ([GHC.CoreExpr] -> Maybe (m (PIRTerm uni fun)))
+knownApps =
+    Map.fromListWithKey (\n -> error ("knownApps: key defined more than once: " <> show n))
+        . fmap (first splitThName)


This is rather odd: you split the name here and also to look up into it... why not just compare the names for equality?

michaelpj · 2023-03-20T11:42:04Z

plutus-tx-plugin/src/PlutusTx/Compiler/Expr.hs

+                            compileExpr . GHC.Var =<< lookupId =<< thNameToGhcNameOrFail 'Builtins.equalsInteger
+                [GHC.Type ty, _numDict, GHC.Lit (GHC.LitNumber _ i), GHC.Lit (GHC.LitNumber _ j)]
+                    | ty `GHC.eqType` GHC.integerTy -> Just $ do
+                        -- If both arguments of `(==) @Integer` are literals, we perform constant folding.


I don't agree with this at all. Keep codegen stupid! Write optimization passes to do optimizations!

michaelpj · 2023-03-21T10:17:56Z

plutus-tx-plugin/test/Budget/allCheap.plc.golden

@@ -23,7 +30,103 @@ letrec
                 !acc : Bool = go xs
               in
               Bool_match
-                 (ifThenElse {Bool} (lessThanEqualsInteger 1 x) False True)
+                 ((let


wow this is terrible D:

michaelpj · 2023-03-21T10:19:25Z

plutus-tx-plugin/test/Budget/allEmptyList.plc.golden

+                    in
+                    Ord_match
+                      {integer}
+                      v


A context-based inlining heuristic would help here: inline if the thing you are inlining is a constructor application and the variable is the scrutinee of a pattern match!

Also need case-of-known-constructor.

michaelpj · 2023-03-21T10:21:09Z

plutus-tx-plugin/test/Budget/show.plc.golden

@@ -270,6 +270,82 @@ letrec
                   {all dead. dead})
            {all dead. dead}
 in
+let


I'm unsure why this one changed so much?

michaelpj · 2023-03-21T10:22:21Z

plutus-tx-plugin/test/Plugin/Errors/literalCaseBs.plc.golden

I guess we should do the trick for == too, but we also probably want to keep the old fallback error which was helpful.

michaelpj · 2023-03-21T10:22:40Z

plutus-tx-plugin/test/Plugin/Errors/literalCaseOther.plc.golden

bezirg · 2023-03-21T10:27:26Z

plutus-tx-plugin/src/PlutusTx/Compiler/Dictionary/Integer.hs

+
+max, min :: Integer -> Integer -> Integer
+{-# INLINEABLE max #-}
+max x y = if x <= y then y else x


wow, I assumed that max is "left-biased" like min, but this is not the case; I learned something today!
Ofcourse you need a "wrong" Eq instance to observe this bias:

data D = D1 | D2 instance Eq D where _ == _ = True -- foul instance deriving instance Ord D deriving instance Show D -- >>> D1 == D2 -- True -- >>> min D1 D2 (leftbiased) -- D1 -- >>> max D1 D2 (rightbiased) -- D2

bezirg · 2023-03-21T10:55:00Z

plutus-tx-plugin/src/PlutusTx/Compiler/Expr.hs

+knownApps ::
+    CompilingDefault uni fun m ann =>
+    Map (Maybe String, String) ([GHC.CoreExpr] -> Maybe (m (PIRTerm uni fun)))
+knownApps =


I think this function could be code-deduplicated slightly

bezirg · 2024-05-17T09:13:12Z

I am not sure if this is still relevant, but here I paste an old comment of mine when I was debugging an older patched GHC (not this one):

Problems encountered:

breaks on banged ! data constructors iff they are defined and compiled in the same module. BangPatterns on expression code works fine however.

Even if we try to avoid relying on ghc base code by using our own prelude, ghc will emit core that implicitly calls some base functions. Implicitly means that even if we never explicitly mentioned base code in Plutus, ghc will emit base code. This happens in at least two cases: a) literal patterns b) (list) stream fusion code.

this is also a problem that should be fixed by the patched-ghc team, by having extensible fields also for base modules.

[prototype] Support base's classes and methods in Plutus Tx

771fa57

zliu41 requested review from bezirg, michaelpj and effectfully March 20, 2023 03:45

zliu41 added the Do not merge label Mar 20, 2023

bezirg reviewed Mar 21, 2023

View reviewed changes

.stylish-haskell.yaml

@@ -11,7 +11,7 @@ steps:

remove_redundant: false

- trailing_whitespace: {}

columns: 100

columns: 99

Copy link

Contributor

bezirg Mar 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

bezirg reviewed Mar 21, 2023

View reviewed changes

michaelpj reviewed Mar 21, 2023

View reviewed changes

bezirg reviewed Mar 21, 2023

View reviewed changes

zeme-wana force-pushed the master branch 2 times, most recently from a161078 to db5cabb Compare July 9, 2024 09:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[prototype] Support `base`'s classes and methods in Plutus Tx #5219

[prototype] Support `base`'s classes and methods in Plutus Tx #5219

zliu41 commented Mar 20, 2023

bezirg commented Mar 21, 2023

bezirg Mar 21, 2023

bezirg Mar 21, 2023

bezirg Mar 21, 2023

bezirg Mar 21, 2023

bezirg Mar 21, 2023

bezirg Mar 21, 2023 •

edited

Loading

michaelpj left a comment

michaelpj Mar 20, 2023

michaelpj Mar 20, 2023

michaelpj Mar 20, 2023

michaelpj Mar 20, 2023

michaelpj Mar 20, 2023

michaelpj Mar 20, 2023

michaelpj Mar 21, 2023

michaelpj Mar 21, 2023

michaelpj Mar 21, 2023

michaelpj Mar 21, 2023

michaelpj Mar 21, 2023

michaelpj Mar 21, 2023

bezirg Mar 21, 2023

bezirg Mar 21, 2023

bezirg commented May 17, 2024 •

edited

Loading

[prototype] Support base's classes and methods in Plutus Tx #5219

Are you sure you want to change the base?

[prototype] Support base's classes and methods in Plutus Tx #5219

Conversation

zliu41 commented Mar 20, 2023

bezirg commented Mar 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bezirg Mar 21, 2023 • edited Loading

Choose a reason for hiding this comment

michaelpj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bezirg commented May 17, 2024 • edited Loading

[prototype] Support `base`'s classes and methods in Plutus Tx #5219

[prototype] Support `base`'s classes and methods in Plutus Tx #5219

bezirg Mar 21, 2023 •

edited

Loading

bezirg commented May 17, 2024 •

edited

Loading