Implement initial cross-compilation support #396
Conversation
| copts = kwargs.get("copts", []) + select({ | ||
| "@platforms//cpu:x86_64": [ | ||
| "-Xcompiler", | ||
| "--target=x86_64-unknown-linux-gnu", |
There was a problem hiding this comment.
This information is already available in the CC toolchain. We need to pass it to the host compiler so that it looks in the right places in the sysroot. But this is probably not the best solution ...
| "strip_prefix": archive_name, | ||
| "version": redist[c_full]["version"], | ||
| }) | ||
| archs = attr.archs or ["x86_64"] |
There was a problem hiding this comment.
The idea here is to iterate over the specified archs and add everything to the specs list.
| "nvcc_version": attr.string( | ||
| doc = "nvcc version. Required for deliverable toolkit only. Fallback to version if omitted.", | ||
| ), | ||
| "archs": attr.string_list(doc = "list of host target architectures to support (e.g., x86_64)"), |
| ) | ||
|
|
||
| cuda = use_extension("@rules_cuda//cuda:extensions.bzl", "toolchain") | ||
| cuda.redist_json( |
There was a problem hiding this comment.
I extended the examples to test this. We will probably need something in tests/integration?
There was a problem hiding this comment.
Yes, we need test, something at least cross compile from x64 to aarch64
| deps = ["@cuda//:cublas"], | ||
| deps = [ | ||
| "@cuda//:cublas", | ||
| "@cuda//:cuda_runtime", |
There was a problem hiding this comment.
This doesn't compile without depending on cuda_runtime. This is a separate issue, though, it has nothing to do with cross compilation.
| doc = "Generate a URL by using the specified version." + | ||
| "This URL will be tried after all URLs specified in the `urls` attribute.", | ||
| ), | ||
| "archs": attr.string_list(doc = "Target architectures to support. If not specified, only x86_64 will be supported."), |
There was a problem hiding this comment.
For attr.components, it now do "If not specified, all components will be used.", I think archs should behave similarly. This also simplify the usage of redist_json quite a bit
There was a problem hiding this comment.
The issue with this is that not all releases provide aarch64 packages. If using "all" means aarch64 and x86_64, some releases (such as 13.0.0) won't work. We could check the JSON file for available archs, so that "all" would include aarch64 only if it's available. However, then the behavior would change depending on the selected release, which could be quite surprising for users.
A further complication is that there is also sbsa, which so far I'm ignoring.
cuda/extensions.bzl
Outdated
| repo_name = redist_json_helper.get_repo_name(module_ctx, spec) | ||
| mapping[spec["component_name"]] = "@" + repo_name | ||
| component_name = spec["component_name"] | ||
| mapping["{}__{}".format(component_name, spec["arch"])] = "@" + repo_name |
There was a problem hiding this comment.
do it in redist_json_helper.get_repo_name
There was a problem hiding this comment.
Do what exactly? Return a tuple with repo name and mapping key? Or add a new function get_mapping_key (I think that's what I'd prefer).
cuda/private/repositories.bzl
Outdated
| nvcc_repo = repository_ctx.attr.components_mapping.get("nvcc", None) | ||
| if not nvcc_repo: | ||
| host_arch = "aarch64" if _is_aarch64(repository_ctx) else "x86_64" | ||
| nvcc_repo = repository_ctx.attr.components_mapping["nvcc__{}".format(host_arch)] |
There was a problem hiding this comment.
Here we need a bit extension. Previously for non cross compiling case, @nvcc are used for both compiling (on exec platform) and linking (for target platform). We need to separate them to @nvcc_the_compiler @nvcc_the_library with better name for sure.
Then your change becomes
nvcc_repo = repository_ctx.attr.components_mapping["nvcc_the_compiler"]
and your hardcoded host_arch goes into a select based on @platforms
There was a problem hiding this comment.
Makes sense. One thing I'm unsure about is how the selection of the correct architecture for the exec platform would work. AFAIU, something like the select I've added to the alias targets of the main repo, cares about the target platform. How exactly can I do this for the exec platform? Sorry if this is obvious, I haven't worked that much with toolchains and the like yet.
There was a problem hiding this comment.
@platform provides host_platform_repo repo rule https://github.com/bazelbuild/platforms/blob/5cf94563e35494b0dab15435868dd7f9e3cab2c8/host/extension.bzl#L53-L60. It seems that we need to optionally setup the @host_platfrom up and then select from it somehow.
There was a problem hiding this comment.
Yeah, "somehow". 😉 I checked toolchains_llvm. They also have to select the correct distribution depending on the host, looks like this happens in repository rules implementation based on repository_ctx. There could be a way to override this (host vs exec platform), but it doesn't look like a select is part of any of this.
There was a problem hiding this comment.
In MODULE.bazel, we need this:
host_platform = use_extension("@platforms//host:extension.bzl", "host_platform")
use_repo(host_platform, "host_platform")
Then in some .bzl or BUILD file we can do this:
load("@host_platform//:constraints.bzl", "HOST_CONSTRAINTS")
On my machine, constraints.bzl looks like this:
# DO NOT EDIT: automatically generated constraints list
HOST_CONSTRAINTS = [
'@platforms//cpu:x86_64',
'@platforms//os:linux',
]
If we could put that on the left side of a select in some nvcc meta-target, we´d be done. But that left side gets evaluated against the target platform, so really not sure how that's supposed to work. I guess we could load this in a .bzl file and then manually implement the select in the repository rule. Would probably be cleaner than using repository_ctx to get that info, but the logic afterwards would remain the same.
There was a problem hiding this comment.
Could it be that I don't need any of this, but instead need to extend the BUILD.toolchain_nvcc template (and code that uses it) to register both, nvcc_x86_64 and nvcc_aarch64, as separate toolchains, but with different exec_compatible_with entries (one with @platforms//cpu:x86_64, the other with @platforms//cpu:aarch64)? And then Bazel would pick the right one automatically depending on the exec platform?
2 participants
Just a very rough draft. For my purposes this might already be good enough, but I'm sure we need to iterate a bit to get this into a mergeable state for this repo. Looking forward to your comments @cloudhan .
#395