Skip to content

ghc: silence error on non-x86_64 with clang #251447

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.

Sign up for GitHub

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

Closed
reckenrode wants to merge 1 commit into from
Closed

ghc: silence error on non-x86_64 with clang #251447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions pkgs/development/compilers/ghc/9.4.2.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,10 @@ stdenv.mkDerivation (rec {
'MinBootGhcVersion="8.10"'
'';

# Silence unused function warning due to `popcount16` being defined on all platforms but used only
# on x86_64. This results in an error due to `-Werror`.
env.NIX_CFLAGS_COMPILE = lib.optionalString (stdenv.cc.isClang && !targetPlatform.isx86_64) "-Wno-unused-function";
Comment on lines +291 to +293
Copy link
Member

@cdepillabout cdepillabout Sep 9, 2023

Choose a reason for hiding this comment

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

This seems reasonable.

I have two questions:

  1. My understanding is that GHC has a single version (or a range of versions) of LLVM that it supports. Would a better fix here be hard-coding the LLVM version used in pkgs/top-level/haskell-packages.nix? We already do this for a few of our compilers, if I recall correctly.
  2. In theory it is alright to merge this before llvmPackages: 11 -> 16 on Darwin and Linux #241692 is merged in? Even if we end up hard-coding the LLVM version in pkgs/top-level/haskell-packages.nix to something earlier than 16, then the changes in this PR won't negatively effect anything, right? This is basically just silencing a warning, right?

Copy link
Contributor Author

@reckenrode reckenrode Sep 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

-Wno-unused-function is supported by at least clang 3. Otherwise, it’d need to be added conditionally based on the clang version (e.g., #253740). Applying this patch without #241692 shouldn’t negatively affect anything.

My understanding is that GHC has a single version (or a range of versions) of LLVM that it supports. Would a better fix here be hard-coding the LLVM version used in pkgs/top-level/haskell-packages.nix? We already do this for a few of our compilers, if I recall correctly.

GHC 9.4.x does specify an LLVM version, but isn’t that for the LLVM backend? When I look at the result of the preConfigure script, it’s setting CC, etc to clang 16.

However, when I drop this patch from my clang 16 branch (to get the specific error and check the build process), it builds successfully. The log shows numerous “errors” related to strict prototypes and this unused function, but they’re being treated as warnings. 🤨

I have no idea what changed or why it started working.

Copy link
Member

@cdepillabout cdepillabout Sep 9, 2023

Choose a reason for hiding this comment

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

GHC 9.4.x does specify an LLVM version, but isn’t that for the LLVM backend?

Ah, I see, so you're saying that there is a difference between the clang version used for compiling GHC, and the version passed as the LLVM backend?

I'm not very familiar with area, so maybe I should let someone else do the review here.

when I drop this patch from my clang 16 branch (to get the specific error and check the build process), it builds successfully

Ah, I see. I wonder what changed for you?

So I guess this PR might end up not being needed?

Copy link
Contributor Author

@reckenrode reckenrode Sep 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Ah, I see, so you're saying that there is a difference between the clang version used for compiling GHC, and the version passed as the LLVM backend?

That appears to be the case.

Ah, I see. I wonder what changed for you?

The only thing that’s changed system-wise is I’m now running the macOS 14 beta, but I don’t know why that would matter.

So I guess this PR might end up not being needed?

Possibly not. I can close it for now and reopen if it causes issues on Hydra. It’s very peculiar. In July, I had all the errors causing problems. When I circled back to test again in August, only GHC 9.4.x was failing (and even it’s building now). That was prior to my installing the macOS 14 beta. I was still on 13.5 at that time.


# TODO(@Ericson2314): Always pass "--target" and always prefix.
configurePlatforms = [ "build" "host" ]
++ lib.optional (targetPlatform != hostPlatform) "target";
Expand Down
4 changes: 4 additions & 0 deletions pkgs/development/compilers/ghc/9.4.3.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,10 @@ stdenv.mkDerivation (rec {
'MinBootGhcVersion="8.10"'
'';

# Silence unused function warning due to `popcount16` being defined on all platforms but used only
# on x86_64. This results in an error due to `-Werror`.
env.NIX_CFLAGS_COMPILE = lib.optionalString (stdenv.cc.isClang && !targetPlatform.isx86_64) "-Wno-unused-function";

# TODO(@Ericson2314): Always pass "--target" and always prefix.
configurePlatforms = [ "build" "host" ]
++ lib.optional (targetPlatform != hostPlatform) "target";
Expand Down
4 changes: 4 additions & 0 deletions pkgs/development/compilers/ghc/9.4.4.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,10 @@ stdenv.mkDerivation (rec {
'MinBootGhcVersion="8.10"'
'';

# Silence unused function warning due to `popcount16` being defined on all platforms but used only
# on x86_64. This results in an error due to `-Werror`.
env.NIX_CFLAGS_COMPILE = lib.optionalString (stdenv.cc.isClang && !targetPlatform.isx86_64) "-Wno-unused-function";

# TODO(@Ericson2314): Always pass "--target" and always prefix.
configurePlatforms = [ "build" "host" ]
++ lib.optional (targetPlatform != hostPlatform) "target";
Expand Down
4 changes: 4 additions & 0 deletions pkgs/development/compilers/ghc/9.4.5.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,10 @@ stdenv.mkDerivation (rec {
'MinBootGhcVersion="8.10"'
'';

# Silence unused function warning due to `popcount16` being defined on all platforms but used only
# on x86_64. This results in an error due to `-Werror`.
env.NIX_CFLAGS_COMPILE = lib.optionalString (stdenv.cc.isClang && !targetPlatform.isx86_64) "-Wno-unused-function";

# TODO(@Ericson2314): Always pass "--target" and always prefix.
configurePlatforms = [ "build" "host" ]
++ lib.optional (targetPlatform != hostPlatform) "target";
Expand Down
4 changes: 4 additions & 0 deletions pkgs/development/compilers/ghc/9.4.6.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,10 @@ stdenv.mkDerivation (rec {
'MinBootGhcVersion="8.10"'
'';

# Silence unused function warning due to `popcount16` being defined on all platforms but used only
# on x86_64. This results in an error due to `-Werror`.
env.NIX_CFLAGS_COMPILE = lib.optionalString (stdenv.cc.isClang && !targetPlatform.isx86_64) "-Wno-unused-function";

# TODO(@Ericson2314): Always pass "--target" and always prefix.
configurePlatforms = [ "build" "host" ]
++ lib.optional (targetPlatform != hostPlatform) "target";
Expand Down