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

rocmPackages: use CMake 3.9.2 when building clr #306375

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

GZGavinZhao
Copy link
Contributor

@GZGavinZhao GZGavinZhao commented Apr 23, 2024

Fixes #305641.

Description of changes

Ensure CMake 3.9.2 is used to build clr, both for ROCm 5 and 6. The changes can be reverted once CMake 3.9.2 lands in unstable, if desired.

I have tested that rocsparse (5 and 6), rocfft (6), and composable_kernel (6) all build.

See #305641 for more details.

cc @mschwaig

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

Signed-off-by: Gavin Zhao <git@gzgz.dev>
Copy link
Member

@mschwaig mschwaig left a comment

Choose a reason for hiding this comment

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

I have reviewed the code and it looks good to me.
The version hashes match the ones from the update to 3.9.2 at #304111, and I have tested that the downloads for the updated source work.

I'm a bit surprised it was really only necessary to change the CMake version used by clr and not anywhere else, but your testing seems to confirm that.

That's enough for me to approve this change.

I am currently still running nixpkgs-review, to check if this change fixes all of the broken packages, and confirm what you saw in your testing.

Since the build is broken on master anyways, if anyone decides they want to just merge this and not wait for nixpkgs-review to finish, go ahead.

@wegank wegank added 12.approvals: 1 This PR was reviewed and approved by one reputable person 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package labels Apr 23, 2024
@mschwaig
Copy link
Member

Result of nixpkgs-review pr 306375 run on x86_64-linux 1

12 packages marked as broken and skipped:
  • rocmPackages.llvm.flang
  • rocmPackages.llvm.flang.doc
  • rocmPackages.llvm.flang.info
  • rocmPackages.llvm.flang.man
  • rocmPackages.migraphx
  • rocmPackages.mivisionx
  • rocmPackages.mivisionx-cpu
  • rocmPackages.mivisionx-hip
  • rocmPackages_5.llvm.flang
  • rocmPackages_5.llvm.flang.doc
  • rocmPackages_5.llvm.flang.info
  • rocmPackages_5.llvm.flang.man
1 package failed to build:
  • rocmPackages_5.rocprofiler
82 packages built:
  • blender-hip
  • magma (magma-hip ,magma_2_7_2)
  • magma.test (magma-hip.test ,magma_2_7_2.test)
  • magma_2_6_2
  • magma_2_6_2.test
  • opensyclWithRocm
  • python311Packages.torchWithRocm
  • python311Packages.torchWithRocm.cxxdev
  • python311Packages.torchWithRocm.dev
  • python311Packages.torchWithRocm.dist
  • python311Packages.torchWithRocm.lib
  • python312Packages.torchWithRocm
  • python312Packages.torchWithRocm.cxxdev
  • python312Packages.torchWithRocm.dev
  • python312Packages.torchWithRocm.dist
  • python312Packages.torchWithRocm.lib
  • rocmPackages.clr
  • rocmPackages.clr.icd
  • rocmPackages.composable_kernel
  • rocmPackages.hipblas
  • rocmPackages.hipcub
  • rocmPackages.hipfft
  • rocmPackages.hiprand
  • rocmPackages.hipsolver
  • rocmPackages.hipsparse
  • rocmPackages.llvm.mlir
  • rocmPackages.miopen
  • rocmPackages.rccl
  • rocmPackages.rocalution
  • rocmPackages.rocblas
  • rocmPackages.rocfft
  • rocmPackages.rocmlir
  • rocmPackages.rocmlir-rock
  • rocmPackages.rocmlir.external
  • rocmPackages.rocprim
  • rocmPackages.rocprofiler
  • rocmPackages.rocr-debug-agent
  • rocmPackages.rocrand
  • rocmPackages.rocsolver
  • rocmPackages.rocsparse
  • rocmPackages.rocthrust
  • rocmPackages.roctracer
  • rocmPackages.rocwmma
  • rocmPackages.rpp (rocmPackages.rpp-hip)
  • rocmPackages.rpp-cpu
  • rocmPackages.rpp-opencl
  • rocmPackages_5.clr
  • rocmPackages_5.clr.icd
  • rocmPackages_5.composable_kernel
  • rocmPackages_5.hipblas
  • rocmPackages_5.hipcub
  • rocmPackages_5.hipfft
  • rocmPackages_5.hiprand
  • rocmPackages_5.hipsolver
  • rocmPackages_5.hipsparse
  • rocmPackages_5.llvm.mlir
  • rocmPackages_5.migraphx
  • rocmPackages_5.miopen (rocmPackages_5.miopen-hip)
  • rocmPackages_5.miopen-opencl
  • rocmPackages_5.miopengemm
  • rocmPackages_5.miopengemm.doc
  • rocmPackages_5.mivisionx (rocmPackages_5.mivisionx-hip)
  • rocmPackages_5.mivisionx-cpu
  • rocmPackages_5.mivisionx-opencl
  • rocmPackages_5.rccl
  • rocmPackages_5.rocalution
  • rocmPackages_5.rocblas
  • rocmPackages_5.rocfft
  • rocmPackages_5.rocmlir
  • rocmPackages_5.rocmlir-rock
  • rocmPackages_5.rocmlir.external
  • rocmPackages_5.rocprim
  • rocmPackages_5.rocr-debug-agent
  • rocmPackages_5.rocsolver
  • rocmPackages_5.rocsparse
  • rocmPackages_5.rocthrust
  • rocmPackages_5.roctracer
  • rocmPackages_5.rocwmma
  • rocmPackages_5.rpp (rocmPackages_5.rpp-hip)
  • rocmPackages_5.rpp-cpu
  • rocmPackages_5.rpp-opencl
  • zluda

I have some unknown issue with the rocprofiler being flaky locally, so this is as good as it gets. LGTM.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/3826

@mschwaig mschwaig requested a review from ulrikstrid April 24, 2024 14:50
@mschwaig
Copy link
Member

@ulrikstrid do you have time to take a look at this?

I just mentioned this PR on Discourse as well, to spread the review load a bit/in case you are busy.
Thanks for all the reviews.

Copy link
Member

@ulrikstrid ulrikstrid left a comment

Choose a reason for hiding this comment

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

I'm okay with this since it unblocks rocm until the cmake update has landed in master

@ulrikstrid ulrikstrid merged commit c062c88 into NixOS:master Apr 24, 2024
27 checks passed
@mschwaig
Copy link
Member

I'm okay with this since it unblocks rocm until the cmake update has landed in master

Great, thanks.

@jonas-w
Copy link
Contributor

jonas-w commented May 6, 2024

Seems like this can be reverted, as 3.29.2 is in unstable:

https://nixpk.gs/pr-tracker.html?pr=304111

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: rocm 10.rebuild-darwin: 1-10 10.rebuild-linux: 101-500 12.approvals: 1 This PR was reviewed and approved by one reputable person 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build failure: rocmPackages, rocmPackages_5
6 participants