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

Enable -fexpose-all-unfoldings #357

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

sjakobi
Copy link
Member

@sjakobi sjakobi commented Mar 1, 2022

This allows us to reliably expose unfoldings for all exposed functions, instead of only those that are marked INLINABLE.

Also remove -fwarn-tabs which is enabled by default now.

sjakobi added 3 commits March 1, 2022 16:11
This allows us to remove a good amount of CPP and hacks.
Stackage snapshots for GHC 8.2 use hashable-1.2.7.0, so this shouldn't
cause any problems.
This allows us to reliably expose unfoldings for all exposed functions,
instead of only those that are marked INLINABLE.

Also remove -fwarn-tabs which is enabled by default now.
@sjakobi
Copy link
Member Author

sjakobi commented Mar 1, 2022

I will finish this after merging #355.

…to silence a warning.
@treeowl
Copy link
Collaborator

treeowl commented Mar 2, 2022

This will definitely do something. Let's make sure it's something good.

@sjakobi
Copy link
Member Author

sjakobi commented Jun 16, 2022

Apparently -fexpose-all-unfoldings does something very different than marking everything INLINEABLE:

https://gitlab.haskell.org/ghc/ghc/-/issues/21715#note_437347

I probably won't work on this further until I have a better understanding of the implications.

@doyougnu
Copy link
Contributor

I also wasn't aware of this difference. I've made a note of it for the handbook.

I don't think -fexpose-all-unfoldings will do a better job than marking the exported functions with INILNEABLE. I would expect INLINEABLE to be better because GHC will have more information at the call site in whichever package is using u-c, and so it has more opportunity to optimize better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants