Skip to content
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

Inline trivial lambda bindings, to beta-reduce later on for smaller scripts #4304

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions plutus-core/plutus-ir/src/PlutusIR/Transform/Inline.hs
Original file line number Diff line number Diff line change
Expand Up @@ -344,11 +344,13 @@ maybeAddTySubst tn rhs = do
-- | Is this a an utterly trivial term which might as well be inlined?
trivialTerm :: Term tyname name uni fun a -> Bool
trivialTerm = \case
Builtin{} -> True
Var{} -> True
Builtin{} -> True
Var{} -> True
-- TODO: Should this depend on the size of the constant?
Constant{} -> True
_ -> False
Constant{} -> True
TyAbs _ _ _ t -> trivialTerm t
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs a note. The decision making here is actually quite complicated. We need to think about two things: saving work, and saving space.

In terms of saving work, I think this is fine. We will create the outermost lambda value at each use site instead of once, but that's a pretty small amount of work that I'm fairly okay with rounding to 0 (but we should document that in a note!). Indeed, several of the other cases also require us to do value creation at each use site, although creating lambda values is probably the most expensive kind (not by much).

In terms of saving space, this is riskier. We will duplicate the whole term at each use site. The other cases all relate to terms which are approximately the same size as a variable (except the constant case! which has a worrying TODO on it...). We could make this much safer by actually doing something more like what the GHC inliner does, and conditionally inlining things at call sites if they're fully applied. But this would be much more complicated...

I think for starters this could do with:

  • A note
  • Splitting up this function into two that represent the two decisions that we're making. Something like costIsAcceptable and sizeIsAcceptable. That way it's clearer what they're checking for and we could in principle use them separately if that's appropriate elsewhere. Then postInlineUnconditional can require that both hold.

Suggested implementations:

costIsAcceptable = \case
  Builtin{} -> True
  Var{} -> True
  Constant{} -> True
  LamAbs{} -> True
  TyAbs{} -> True
  Error{} -> True
  IWrap _ _ _ t -> costIsAcceptable t
  Unwrap _ t -> costIsAcceptable t

  Apply{} -> False
  TyInst{} -> False
  Let{} -> False

sizeIsAcceptable = \case
  Builtin{} -> True
  Var{} -> True
  -- These are the two dubious cases, note here
  LamAbs _ _ _ t -> sizeIsAcceptable t
  TyAbs _ _ _ t -> sizeIsAcceptable t
  Error{} -> True
  
  Constant{} -> False --- I actually think inlining constants is pretty dubious 
  -- Arguably we could allow these two...
  IWrap _ _ _ t -> False
  Unwrap _ t -> False
  Apply{} -> False
  TyInst{} -> False
  Let{} -> False

LamAbs _ _ _ t -> trivialTerm t
_ -> False

-- | Is this a an utterly trivial type which might as well be inlined?
trivialType :: Type tyname uni a -> Bool
Expand Down
2 changes: 1 addition & 1 deletion plutus-core/plutus-ir/test/transform/inline/single
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@

(termbind
(strict)
(vardecl noInline (con integer))
(vardecl trivialLambda (con integer))
(let
(nonrec)
(termbind
Expand Down
12 changes: 2 additions & 10 deletions plutus-core/plutus-ir/test/transform/inline/single.golden
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,8 @@
)
(termbind
(strict)
(vardecl noInline (con integer))
(let
(nonrec)
(termbind
(strict)
(vardecl f (fun (con integer) (con integer)))
(lam y (con integer) y)
)
[ f [ f (con integer 1) ] ]
)
(vardecl trivialLambda (con integer))
[ (lam y (con integer) y) [ (lam y (con integer) y) (con integer 1) ] ]
)
(termbind
(strict)
Expand Down
2 changes: 0 additions & 2 deletions plutus-tx-plugin/src/PlutusTx/Plugin.hs
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,6 @@ data PluginOptions = PluginOptions {

-- Setting to `True` defines `trace` as `\_ a -> a` instead of the builtin version.
-- Which effectively ignores the trace text.
-- TODO: when simpilers are enabled, we should reduce and inline a `\_ a -> a` call to just `a`.
-- Reducing `test/Plugin/NoTrace/traceComplex.plc.golden` is a good start.
, poRemoveTrace :: Bool
}

Expand Down
16 changes: 2 additions & 14 deletions plutus-tx-plugin/test/Plugin/NoTrace/traceComplex.plc.golden
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,13 @@
(datatypebind
(datatype (tyvardecl Unit (type)) Unit_match (vardecl Unit Unit))
)
(termbind
(strict)
(vardecl trace (all a (type) (fun (con string) (fun a a))))
(abs a (type) (lam t (con string) (lam a a a)))
)
(lam
ds
Bool
{
[
[
{ [ Bool_match ds ] (all dead (type) Unit) }
(abs dead (type) [ [ { trace Unit } (con string "yes") ] Unit ])
{ [ Bool_match ds ] (all dead (type) Unit) } (abs dead (type) Unit)
]
(abs
dead
Expand All @@ -34,13 +28,7 @@
(termbind
(strict)
(vardecl thunk (con unit))
[
{
[ Unit_match [ [ { trace Unit } (con string "no") ] Unit ] ]
(con unit)
}
(con unit ())
]
[ { [ Unit_match Unit ] (con unit) } (con unit ()) ]
)
(error Unit)
)
Expand Down