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

u-c: cabal: add no-msse4.2 flag, enable -msse4.2 as default on x86 architectures #483

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

doyougnu
Copy link
Contributor

@doyougnu doyougnu commented Sep 12, 2023

closes #464.

I didn't test this locally so we'll see what CI says.

Also I assumed this would be in a new minor release so I added that to the CHANGE.md file but did not change the versioning in the cabal file

@doyougnu doyougnu force-pushed the doyougnu/add-no-msse-flag branch 2 times, most recently from 757f900 to 45cc38f Compare September 12, 2023 21:26
Copy link
Contributor

@konsumlamm konsumlamm left a comment

Choose a reason for hiding this comment

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

I think it would make more sense to call the flag sse42 (the m isn't part of the name, it's just a prefix for machine-specific options) and enable it by default.

unordered-containers.cabal Outdated Show resolved Hide resolved
unordered-containers.cabal Outdated Show resolved Hide resolved
@doyougnu doyougnu force-pushed the doyougnu/add-no-msse-flag branch from 661ec4c to 9a449f2 Compare September 13, 2023 12:56
unordered-containers.cabal Outdated Show resolved Hide resolved
unordered-containers.cabal Outdated Show resolved Hide resolved
@doyougnu doyougnu force-pushed the doyougnu/add-no-msse-flag branch from b2d9772 to ea9dd46 Compare September 13, 2023 14:19
@doyougnu doyougnu force-pushed the doyougnu/add-no-msse-flag branch from 1bd9fa3 to ad52f90 Compare September 13, 2023 14:25
@phadej
Copy link
Contributor

phadej commented Oct 10, 2023

I'd suggest OP use some global version of ghc-options: -msse4.2, rather than adding it to individual packages. Adding to packages doesn't scale. I opened a GHC issue https://gitlab.haskell.org/ghc/ghc/-/issues/24073

@TeofilC
Copy link

TeofilC commented Apr 25, 2024

unordered-containers is quite inlining heavy, eg, I often see lookups inlined quite heavily when looking at Core. If the popcount call ends up being inlined when compiling another package (that isn't passed -msse4.2) will this flag still apply?

@phadej
Copy link
Contributor

phadej commented Apr 25, 2024

@TeofilC

will this flag still apply?

Good question. I think it wont, as at Core level (i.e. in unfoldings to be inlined) we only have magic popCount# primop, and the code generation happens later.

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.

u-c should use -msse4.2 by default
4 participants