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

Plutus plugin should automatically mark all functions and instance methods as inlinable #4277

Open
mgajda opened this issue Dec 8, 2021 · 12 comments
Labels
enhancement Low priority Doesn't require immediate attention status: triaged

Comments

@mgajda
Copy link

mgajda commented Dec 8, 2021

Describe the feature you'd like

Most of the functions used in Plutus program need to be market as INLINABLE to be used outside the module.
This is inconvenient. Moreover it is insufficient in some cases.
For example, automatically derived class instances cannot be fixed this way.

Describe alternatives you've considered

  1. Defining all instances manually is a bit silly for large ADTs:
instance Eq Bool where
  {-# INLINABLE #-}
  (==) = ...
  1. Marking all functions as inlinable also adds unnecessary code when the code is already processed by Plutus plugin.
  2. We could add plugin with GHC_OPTION that marks all functions, including derived instances as inlinable.

Describe the approach you would take to implement this

I would love to talk with Plutus plugin author and get instructions on how to best add it.

@L-as
Copy link
Contributor

L-as commented Dec 9, 2021

s/may/should?

@michaelpj
Copy link
Contributor

I believe we can't do this with plugins, but I might be wrong. Plus, you would need the INLINABLE-adding plugin to run on the modules you use, not just the ones where you compile the final PLC, which is much more difficult to set up.

@L-as
Copy link
Contributor

L-as commented Dec 9, 2021

I think the streamly plugin does this with INLINE FWIW. See https://github.com/composewell/fusion-plugin

@michaelpj
Copy link
Contributor

I think that sets INLINE so long as the binding already has an unfolding. I don't think we can create an unfolding if one doesn't exist. And we'd still have to run the plugin on the upstream libraries to achieve that also.

@mgajda
Copy link
Author

mgajda commented Dec 9, 2021

The current Plutus setup compiles libraries or brings them from IOHK cache anyway. Can we connect this plugin and simply rebuild entire library once? Or mark certain modules as used by Plutus and rebuild them with GHC having new compiler option?

@mgajda mgajda changed the title Plutus plugin may automatically mark all functions and instance methods as inlinable Plutus plugin should automatically mark all functions and instance methods as inlinable Dec 9, 2021
@effectfully
Copy link
Contributor

@michaelpj has something changed since the issue was opened? Does -fexpose-all-unfoldings now help us alleviate the problem? Is there anything we could do here?

@effectfully effectfully added the status: needs triage GH issues that requires triage label Feb 1, 2023
@michaelpj
Copy link
Contributor

I don't know of a way to improve the current situation. My best ideas all involve substantial rework of the entire system :(

@effectfully
Copy link
Contributor

I don't know of a way to improve the current situation. My best ideas all involve substantial rework of the entire system :(

I see. So this is something that would be great to have, but requires a huge amount of effort. Marking it with "Low priority" then.

@effectfully effectfully added Low priority Doesn't require immediate attention status: triaged and removed status: needs triage GH issues that requires triage labels Feb 2, 2023
@uhbif19
Copy link

uhbif19 commented Jun 26, 2023

I don't know of a way to improve the current situation. My best ideas all involve substantial rework of the entire system :(

I do not understand. What is need to be reworked?

If haskell stock Eq/etc will be used with -fexpose-all-unfoldings - will it work on-chain? It is now forbidden by plugin, but I mean if disable this check.

@michaelpj
Copy link
Contributor

-fexpose-all-unfoldings exposes optimized unfoldings, which are often unusable because GHC's optimization pipeline performs optimizations that assume e.g. the target platform, stuff about foreign pointers and so on. We're fairly reliant on getting pre-optimization Core.

Ultimately this is our big problem: how do we get the core for bindings in other modules? We are getting by with the current approach, but it's fragile and many things can make it go wrong. We never got very far with trying to get something into GHC to help. So: a big idea is needed IMO.

@L-as
Copy link
Contributor

L-as commented Jul 15, 2023

@michaelpj Can't you just disable the optimisations then?

@michaelpj
Copy link
Contributor

a) not enough, and b) because of the way we Plutus Tx works, that also means turning off optimization for normal Haskell code compiled in that compilation, which is not what you want.

There's also the fact that many things in base have non-elementary definitions, e.g. there is stuff that uses dataToTag instead of having a pattern-matching definition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Low priority Doesn't require immediate attention status: triaged
Projects
None yet
Development

No branches or pull requests

5 participants