-
Notifications
You must be signed in to change notification settings - Fork 2
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
base: main
Are you sure you want to change the base?
Conversation
b7bb1cd
to
1b35cd2
Compare
It is not entirely clear to me that this fixes the problem we had. We never set Could you test this on Heavisoft? We re-implemented |
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). |
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. |
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 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. |
That's weird. With the removal of presimplifier, now no simplifier runs
before the plugin, so there shouldn't be any premature simplification. I'll
look into it
…On Sat, Apr 16, 2022, 3:06 PM Greg Pfeil ***@***.***> wrote:
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.
—
Reply to this email directly, view it on GitHub
<#56 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABQMPCTP32UZHOTABTH4XJLVFM2X5ANCNFSM5TRN32TQ>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
1b35cd2
to
ff0ba80
Compare
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.
ff0ba80
to
340f62e
Compare
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.