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

Add a DynFlags plugin. #56

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Add a DynFlags plugin. #56

wants to merge 1 commit into from

Conversation

sellout
Copy link
Member

@sellout sellout commented Apr 15, 2022

This only has an effect on GHC 8.10 and later. Older versions need to manually
set the relevant flags.

TODO: warn when we had to override flags set by the user (ideally, we'd only
warn when they explicitly differ, not when we're overriding behavior set by
-O2 or something.

This also currently doesn't work, because some of our tests break when we don't
ignore interface pragmas (I was hoping this would go away when we removed the
presimplifier, but it didn't).

Fixes #43.

@sellout sellout force-pushed the dynflags-plugin branch 3 times, most recently from b7bb1cd to 1b35cd2 Compare April 16, 2022 01:49
@zliu41
Copy link
Member

zliu41 commented Apr 16, 2022

It is not entirely clear to me that this fixes the problem we had. We never set -funbox-strict-fields; it's the linear package that sets it.

Could you test this on Heavisoft? We re-implemented cross to avoid that problem, and you can remove that implementation and turn on this plugin and see what happens.

@zliu41
Copy link
Member

zliu41 commented Apr 16, 2022

Also, any idea why some tests require ignoring interface pragmas? I thought interface pragmas should never be ignored in practice (otherwise the plugin wouldn't be able to inline anything).

@sellout
Copy link
Member Author

sellout commented Apr 16, 2022

It is not entirely clear to me that this fixes the problem we had. We never set -funbox-strict-fields; it's the linear package that sets it.

Could you test this on Heavisoft? We re-implemented cross to avoid that problem, and you can remove that implementation and turn on this plugin and see what happens.

Yeah, I think you're right, this is unlikely to help here. Shouldn't have added it. Was mostly trying to rescue various WIPs from Gerrit. And added that flag in vain hope.

@sellout
Copy link
Member Author

sellout commented Apr 16, 2022

Also, any idea why some tests require ignoring interface pragmas? I thought interface pragmas should never be ignored in practice (otherwise the plugin wouldn't be able to inline anything).

I haven't touched this change in like a year, so I don't recall. I know that initially I fixed some of the issues that were getting in the way of -no-ignore-interface-pragmas, but clearly we never got past them all.

The tests that fail are all ones where we expect to interpret the expression directly, so presumably the interface pragma is resulting in some premature inlining, specialization, or something.

@zliu41
Copy link
Member

zliu41 commented Apr 16, 2022 via email

@sellout sellout changed the base branch from master to main February 8, 2024 16:05
This only has an effect on GHC 8.10 and later. Older versions need to manually
set the relevant flags.

TODO: warn when we had to override flags set by the user (ideally, we'd only
warn when they explicitly differ, not when we're overriding behavior set by
`-O2` or something.

This also currently doesn't work, because some of our tests break when we don't
ignore interface pragmas (I was hoping this would go away when we removed the
presimplifier, but it didn't).

Fixes #43.
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.

Avoid requiring -fno-ignore-interface-pragmas on Categorifier-using targets
2 participants