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

rocm-runtime is still using gcc, is about to become broken by gcc13 #271943

Closed
ghost opened this issue Dec 3, 2023 · 8 comments · Fixed by #271954
Closed

rocm-runtime is still using gcc, is about to become broken by gcc13 #271943

ghost opened this issue Dec 3, 2023 · 8 comments · Fixed by #271954

Comments

@ghost
Copy link

ghost commented Dec 3, 2023

Heads up: even though it tries to overrideCC with clang, our rocm-runtime expression still uses gcc for a ton of stuff.

Try changing default-gcc-version in pkgs/top-level/all-packages.nix to 13 and you'll see the breakage. I'm about to merge

which does this, and it's going to break rocmPackages. I tried to fix it, but the problem is very deep: you guys think you're using only clang, but in fact you're not.

Wanted to give you a heads-up.

cc: @lovesegfault @Madouura @Flakebi

@ghost ghost added the 0.kind: bug label Dec 3, 2023
@ghost
Copy link
Author

ghost commented Dec 3, 2023

I recommend you guys merge a PR which pins rocmPackages to gcc12Stdenv wherever you aren't really using llvm/clang. I tried to do this, but it's far from clear where this needs to be done.

@ghost
Copy link
Author

ghost commented Dec 3, 2023

Also thank you all for the awesome hard work you did to make this happen. I was totally shocked when I was able to get 8*MI25s to run Falcon-180B in llama.cpp with only a few days of effort. Obviously that's because you folks had already put in months of work to make it so smooth...

@Madouura
Copy link
Contributor

Madouura commented Dec 3, 2023

you guys think you're using only clang, but in fact you're not.

Unfortunately, I'm aware. I was working on making rocmPackages use only libc/libcxx/etc, but it's rather WIP.

I tried to do this, but it's far from clear where this needs to be done.

It's likely all the way to the root - ROCm LLVM. It has a GCC backend which is used.

Any chance you could give me the error log for rocm-runtime so I don't have to rebuild ROCm LLVM and a bunch of other stuff?

@ghost
Copy link
Author

ghost commented Dec 3, 2023

Unfortunately, I'm aware.

Ah, that's actually somewhat of a relief. At least the problem is known.

Log is attached. I'm happy to test fixes; I have plenty of compute. After the new year I will be working on making the buildfarm available to other projects; you'll be on the shortlist if you're interested.
8q1vy1pyk4079vjq3pfqkjmffdvksif2-rocm-llvm-runtimes-5.7.1.drv.log.gz

@ghost
Copy link
Author

ghost commented Dec 3, 2023

Also if you need to get my attention I hang out in #tvl on irc.hackint.org. It's bridged to Matrix.

@Madouura
Copy link
Contributor

Madouura commented Dec 3, 2023

8q1vy1pyk4079vjq3pfqkjmffdvksif2-rocm-llvm-runtimes-5.7.1.drv.log.gz

Okay, this is rocm-llvm-runtimes, not rocm-runtime. This might be a slightly easier fix then.

I'm happy to test fixes; I have plenty of compute. After the new year I will be working on making the buildfarm available to other projects; you'll be on the shortlist if you're interested.

I appreciate that, when I put the PR through I might request that you do a nixpkgs-review, possibly a few times if the dependency hell is really bad.

@Madouura
Copy link
Contributor

Madouura commented Dec 4, 2023

Actually, I think I may take your suggestion and just pin a GCC version until a new ROCm release.
This will likely be fixed upstream.

ghost pushed a commit to Madouura/nixpkgs that referenced this issue Dec 5, 2023
Try reverting this on a future ROCm release.

Addresses: NixOS#271943
ghost pushed a commit that referenced this issue Dec 5, 2023
Try reverting this on a future ROCm release.

Addresses: #271943
@Madouura
Copy link
Contributor

Madouura commented Dec 5, 2023

Fixed by #271954
(Didn't auto close for some reason?)

@Madouura Madouura closed this as completed Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants