-
Notifications
You must be signed in to change notification settings - Fork 10
support ghc-flavor ghc-9.8.4 #571
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
support ghc-flavor ghc-9.8.4 #571
Conversation
dylant-da
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you for this!
I'll contact Richard on our end to ask about the workflow changes.
|
Looks like gitlab failed to resolve, possibly a flake, restarting |
i just saw a post to the GHC Devs list about https://ghc.gitlab.haskell.org gives 502 bad gateway for Liquid Haskell so we are not the only project affected. |
| when (ghcFlavor == Ghc984) $ | ||
| let file = "compiler/cbits/genSym.c" | ||
| in writeFile file | ||
| . replace | ||
| "HsWord64 u = atomic_inc64" | ||
| "HsWord64 u = atomic_inc" | ||
| =<< readFile' file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition for this change seems odd to me. Why ==?
I have two datapoints:
- ghc-9.8.5 support #585 - which adds
Ghc985, but didn't adjust this condition. Should it become>=to also cover GHC 9.8.5? - We have build failures for ghc-lib-parser-9.8.5.20250214 in nixpkgs for:
They all fail the same way:
compiler/cbits/genSym.c: In function ‘ghc_lib_parser_genSym’:
compiler/cbits/genSym.c:46:18: error:
error: implicit declaration of function ‘atomic_inc64’; did you mean ‘atomic_inc’? [-Wimplicit-function-declaration]
46 | HsWord64 u = atomic_inc64((StgWord64 *)&ghc_unique_counter64, ghc_unique_inc) & UNIQUE_MASK;
| ^~~~~~~~~~~~
| atomic_inc
|
46 | HsWord64 u = atomic_inc64((StgWord64 *)&ghc_unique_counter64, ghc_unique_inc) & UNIQUE_MASK;
| ^
`gcc' failed in phase `C Compiler'. (Exit code: 1)
This looks to me like the same change required.
Should this replacement happen for all GHC 9.8.x?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Wolfgang, that looks to be the case, yes, thank you for digging and finding the underlying issue here - I'll take a look at putting together a PR for this. To help my testing, how would I reproduce the Nix build failure you're having locally for 9.8.1/2/3?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested a bit, and it seems to me that:
- GHC 9.8.4 doesn't care whether we have
atomic_incoratomic_inc64. - GHC 9.8.3 only likes
atomic_inc.
(that is the GHC that is building ghc-lib-parser)
ghc-lib-parser-9.8.5.20250214 was created with the 985 flavor, which doesn't do the replacement. Thus, it works for us in nixpkgs with GHC 9.8.4, but not with those below.
Also, ghc-lib-parser-9.8.3.20241103 doesn't need the replacement, because the bootstrapping already created atomic_inc code to start with.
Thus, answering my own question:
Should this replacement happen for all GHC 9.8.x?
No, it should not. But it should happen for GHC 9.8.4 and up. So the condition should be ghcFlavor >= Ghc984.
Note, that I don't think you need to use nix to recreate this. You just need to try to build ghc-lib-parser-9.8.5.20250214 with GHC 9.8.3, this should also work with e.g. ghcup or so.
If you do want to use Nix, I ran it with this in the ghc-lib repo:
nix-shell -p haskell.compiler.ghc983 -p cabal-install -p python3 -p autoconf -p automake
and then I ran:
cabal run exe:ghc-lib-build-tool -- --ghc-flavor ghc-9.8.5
add support for ghc-9.8.4
atomic_inc64withatomic_incin 'genSym.c' ((i) pre 9.8.4 RTS don't haveatomic_inc64(ii)atomic_incdid for 9.8.3 it will do here (iii)atomic_incon 64-bit platforms appears equivalent in effect (iv) all of this is eliminated from 'genSym.c' in later versions of GHC)delete .github/workflows/ghc-9.8.2-ghc-9.6.5.yml and replace it with .github/workflows/ghc-9.8.4-ghc-9.6.5.yml
i also tweaked the
filepathlower bounds in the 9.8 and 9.10 cases (making them more consistent with GHC)