cudaPackages: enable cross-compilation (take two) #279952

ConnorBaker · 2024-01-10T02:36:41Z

Important

This PR includes changes which have not yet been merged into master. It should not be merged prior to them:

gcc: link $lib/lib -> $lib/$targetConfig correctly and consistently #296219
- NOTE: This PR targeted staging and must be merged into master before merging.
stdenvAdapters.useLibsFrom: use targetStdenv.cc.override #281371

Description of changes

#275560 brings in a bunch of additional work that I thought needed to be coupled with support for cross-compilation, but over the break I decided to revisit this. Maybe it doesn't need to be coupled -- this is a second attempt at enabled cross-compilation for cudaPackages.

Of note, both cuda-modules/setup-hooks/extension.nix and cuda-modules/cuda/overrides.nix have been refactored. They are now attribute sets of functions which are invoked with callPackage to create setup hooks and package overrides, respectively. The move to using final.callPackage rather than retrieving required packages directly from final allows us to use the __spliced attribute on derivations when cross-compiling to ensure we're selecting the correct version of a package.

When adding packages to nativeBuildInputs or buildInputs, splicing (largely) happens automatically. That is, there is no need to manually specify which splice a package should be drawn from. Additionally, the default package set the callPackage arguments are drawn from, pkgs (aka pkgsHostTarget), helps minimize breakage. However, there are places where we do need to specify the splice to use:

Using specific package outputs: splices must be manually inserted prior to using a particular output of a package, as otherwise the output will be drawn from the splice corresponding to pkgs (which is to say pkgsHostTarget).
Phases: any usage of a package supplied by nativeBuildInputs in pre/post phases should be spliced so the correct package is made available.
Flags: any usage of a package supplied by nativeBuildInputs in a *Flags-style argument should be spliced, again to ensure the correct package is made available and that the default (pkgs) splice isn't used instead.

Things done

Add a 👍 reaction to pull requests you find important.

pkgs/development/cuda-modules/generic-builders/manifest.nix

pkgs/development/cuda-modules/cuda/overrides.nix

ConnorBaker · 2024-01-23T03:49:12Z

pkgs/development/cuda-modules/cuda/overrides.nix

-            --replace \
-              '$(TOP)/$(_TARGET_DIR_)/include' \
-              "''${!outputDev}/include"
+        # TODO(@connorbaker): We should specify the spliced version of backendStdenv and cuda_cudart to use here.


TODO @ConnorBaker

ConnorBaker · 2024-01-23T03:51:11Z

pkgs/development/cuda-modules/setup-hooks/extension.nix

+        substitutions = {
+          # Required in addition to ccRoot as otherwise bin/gcc is looked up
+          # when building CMakeCUDACompilerId.cu
+          ccFullPath = "${backendStdenv.cc}/bin/${backendStdenv.cc.targetPrefix}c++";
+          # Point NVCC at a compatible compiler
+          ccRoot = "${backendStdenv.cc}";
+          setupCudaHook = placeholder "out";
+        };


TODO @ConnorBaker

Are these correct? Do we need to use a spliced version? Unclear whether makeSetupHook does anything on the backend to draw these from buildPackages (pkgsBuildHost) or uses the default pkgs (pkgsHostTarget).

Thinking locally, yes they should be spliced. The hook is going to reside in the target derivation's nativeBuildInputs, so with splicing I expect the hook to be taken from the buildPackages. I expect the backendStdenv from that splice to contain a compiler for build->build instead of build->host.

Thinking more globally, there are just so many places where we already hard-code the cc at evaluation/rendering time: setupCudaHook, nvcc, backendStdenv. This wrong, redundant, complex, and we better choose just one place

We should use the current host's compiler. Then we take the hook from the BuildHost slice, so its "host" is really our "build"

ConnorBaker · 2024-01-23T03:55:19Z

@yannham current blockers are these:

Building cuda_nvcc will fail because the order of the outputs matters, and lib should not come after static as it will move all the libraries out of the static output and into the lib output, causing an empty static output and a build failure: #279952 (comment)

Fixing that error (either by commenting it out temporarily or inserting lib in the correct position) leads to a new build error when trying to compile legacyPackages.x86_64-linux.pkgsCross.aarch64-multiplatform.cudaPackages.saxpy -- cuda_nvcc from the Jetson (linux-aarch64 is the NVIDIA redist arch name) tarball being built on x86_64-linux fails to link against the libgcc and libc++ libraries from the buildPackages compiler (as expected). However, I have no idea why it's trying to do that.

Unclear whether we're sourcing compilers from the correct package sets in #279952 (comment) and #279952 (comment).

SomeoneSerge · 2024-01-23T13:45:43Z

the order of the outputs matters, and lib should not come after static as it will move all the libraries out of the static output and into the lib output

That's a regression after the cuda12 fixes then, and we should PR a fix separately. I thought we'd just update the manifest generator and remove these outputs = ... ++ ... hacks

SomeoneSerge · 2024-01-23T14:35:12Z

pkgs/development/cuda-modules/generic-builders/manifest.nix

@@ -46,7 +46,7 @@ let
  # redistArch :: String
  # The redistArch is the name of the architecture for which the redistributable is built.
  # It is `"unsupported"` if the redistributable is not supported on the target platform.
-  redistArch = flags.getRedistArch hostPlatform.system;
+  redistArch = flags.getRedistArch targetPlatform.system;


Why? I think we generally want the host system's binaries, except when we handle lib/ and nvvm/lib/ in nvcc (then we do want target)

pkgs/development/cuda-modules/setup-hooks/extension.nix

ConnorBaker · 2024-03-20T03:59:16Z

TODO(@ConnorBaker): NVCC, as well as any dependency which finds itself in nativeBuildInputs, cannot have a dependency on cuda_compat (usually introduced by way of one of our setup hooks) when we are cross-compiling.

Likely for ease of implementation, NVCC is used as a way to inject dependencies on setup hooks. If that's the case, it falls apart when cross-compiling, as our setup hooks need to be able to run on the build platform rather than the host/target.

…pattern

Since, even under cross-compilation, we evaluate this flag on multiple platforms, it makes more sense to move the platform check out of the throw condition and into the boolean return value. The alternative is to restrict all uses of this value to locations which gaurd evaluation so it does not occur when the host platform is still x86_64.

…veBuildInputs

…ostPatch phase

…body like other setup hooks

…to cuda_cudart

…nable structuredAttrs

… output

ConnorBaker · 2024-04-04T02:28:36Z

Closing as this PR is largely superseded by #301416.

ConnorBaker added the 6.topic: cuda Parallel computing platform and API label Jan 10, 2024

ConnorBaker self-assigned this Jan 10, 2024

github-actions bot added the 6.topic: python label Jan 10, 2024

ofborg bot added the 6.topic: cross-compilation Building packages on a different platform than they will be used on label Jan 10, 2024

ConnorBaker changed the title ~~cudaPackages: enable cross-compilation~~ cudaPackages: enable cross-compilation (take two) Jan 10, 2024

This comment was marked as outdated.

Sign in to view

SomeoneSerge reviewed Jan 12, 2024

View reviewed changes

pkgs/development/cuda-modules/generic-builders/manifest.nix Show resolved Hide resolved

ConnorBaker mentioned this pull request Jan 12, 2024

cudaPackages.backendStdenv.cc: gccForLibs #275947

Merged

13 tasks

This comment was marked as outdated.

Sign in to view

ConnorBaker force-pushed the feat/cudaPackages-cross-compilation branch from e371d54 to a2d5dcc Compare January 16, 2024 14:49

github-actions bot removed the 6.topic: python label Jan 16, 2024

This comment was marked as outdated.

Sign in to view

ConnorBaker force-pushed the feat/cudaPackages-cross-compilation branch 2 times, most recently from 7b5683f to 5a9d2c8 Compare January 23, 2024 03:36

github-actions bot added the 6.topic: stdenv Standard environment label Jan 23, 2024

ConnorBaker force-pushed the feat/cudaPackages-cross-compilation branch from 5a9d2c8 to ecb9e96 Compare January 23, 2024 03:43

ConnorBaker commented Jan 23, 2024

View reviewed changes

pkgs/development/cuda-modules/cuda/overrides.nix Outdated Show resolved Hide resolved

ConnorBaker commented Jan 23, 2024

View reviewed changes

SomeoneSerge reviewed Jan 23, 2024

View reviewed changes

pkgs/development/cuda-modules/setup-hooks/extension.nix Outdated Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

ConnorBaker force-pushed the feat/cudaPackages-cross-compilation branch from 2dbe0a5 to 2525205 Compare March 19, 2024 21:25

This comment was marked as outdated.

Sign in to view

Connor Baker added 18 commits March 26, 2024 13:58

cuda-modules/cuda/overrides: simplify callPackage then overrideAttrs …

0fec676

…pattern

cudaPackages.cuda_nvcc: lib must precede static in outputs

ddcfff0

cuda-modules: add check for duplicate/misordered outputs

112d38b

cuda-modules: update note on use of lndir from path

49676c7

cudaPackages.saxpy: Jetson should be supported after CUDA 11.4

868fa52

cuda-modules/cuda/overrides: remove unused callPackage arguments

2b351b2

cuda-modules/flags: use cudaAtLeast when possible

0304c9b

cuda-modules: fix deprecated uses of substituteInPlace replace flag

969ee2b

cuda-modules/cuda/overrides: backendStdenv.cc is already part of nati…

3b629a2

…veBuildInputs

cuda-modules/cuda/overrides: specify spliced packages for cuda_nvcc p…

e55a9c2

…ostPatch phase

cuda-modules/generic-builders/manifest: wip cross-compilation

f85d321

cuda-modules/saxpy: remove CMAKE_VERBOSE_MAKEFILE

2b6a5a9

cuda-modules/setup-hooks/setup-cuda-hook: factor out cc access

77ea14b

cuda-modules/setup-hooks/auto-add-cuda-compat-runpath-hook: collapse …

37d2448

…body like other setup hooks

cuda-modules/setup-hooks: wip rewrite and set NIX_DEBUG=1

de0bb6a

cudaPackages.cuda_nvcc: never has a lib output

3297d6f

cuda-modules/setup-hooks: wip

1ac7621

ConnorBaker force-pushed the feat/cudaPackages-cross-compilation branch from 1a9f28b to 1ac7621 Compare March 26, 2024 13:58

Connor Baker added 7 commits March 27, 2024 03:02

cuda-modules/cuda/overrides: cuda_nvcc should not include references …

a026c05

…to cuda_cudart

cudaPackages.nccl: remove unneeded makeFlags, specify splicing, and e…

b340c3f

…nable structuredAttrs

cuda-modules/generic-builders/manifest: cleanup

0fa534c

cuda-modules/setup-hooks/mark-for-cudatoolkit-root-hook: rewrite

1f077ac

cudaPackages.saxpy: getDev/getLib would not always select the desired…

3fc9e9b

… output

cuda-modules/setup-hooks/setup-cuda-hook: rewrite

7fced11

cudaPackages.saxpy: attempt manually setting flags for cross

b43bf06

ConnorBaker mentioned this pull request Apr 4, 2024

cuda-modules: apply lessons learned from cross-compilation attempts #301416

Merged

17 tasks

ConnorBaker closed this Apr 4, 2024

cole-h removed the ofborg-internal-error Ofborg encountered an error label Apr 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cudaPackages: enable cross-compilation (take two) #279952

cudaPackages: enable cross-compilation (take two) #279952

ConnorBaker commented Jan 10, 2024 •

edited

Loading

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

ConnorBaker Jan 23, 2024

ConnorBaker Jan 23, 2024

SomeoneSerge Jan 23, 2024

SomeoneSerge Mar 20, 2024

ConnorBaker commented Jan 23, 2024

SomeoneSerge commented Jan 23, 2024

SomeoneSerge Jan 23, 2024

This comment was marked as outdated.

This comment was marked as outdated.

ConnorBaker commented Mar 20, 2024

ConnorBaker commented Apr 4, 2024

cudaPackages: enable cross-compilation (take two) #279952

cudaPackages: enable cross-compilation (take two) #279952

Conversation

ConnorBaker commented Jan 10, 2024 • edited Loading

Description of changes

Things done

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

ConnorBaker Jan 23, 2024

Choose a reason for hiding this comment

ConnorBaker Jan 23, 2024

Choose a reason for hiding this comment

SomeoneSerge Jan 23, 2024

Choose a reason for hiding this comment

SomeoneSerge Mar 20, 2024

Choose a reason for hiding this comment

ConnorBaker commented Jan 23, 2024

SomeoneSerge commented Jan 23, 2024

SomeoneSerge Jan 23, 2024

Choose a reason for hiding this comment

This comment was marked as outdated.

This comment was marked as outdated.

ConnorBaker commented Mar 20, 2024

ConnorBaker commented Apr 4, 2024

ConnorBaker commented Jan 10, 2024 •

edited

Loading