[sgasho]

Closes #164057
Follow-up to: #164455

The host pass now takes the linked device output (<crate>.ptx for NVPTX, <crate>.elf for AMDGPU) instead of the device bitcode.

The device crate must now be built as a cdylib. rustc only passes --export-symbol to llvm-bitcode-linker for cdylibs, so a bin would lose its kernels to internalization.

I had to add [lib] in Cargo.toml

[lib]
name = "sample_offload"
path = "src/main.rs"
crate-type = ["cdylib"]

Without it, the runtime fails at startup

PluginInterface error: Failure to init kernel: "requested object was not found in the binary image" error in cuModuleGetFunction('_RNvC14sample_offload6kernel'): named symbol not found
omptarget error: Failed to load kernel _RNvC14sample_offload6kernel

I’m not sure whether bin device crates should export their kernels too, so I left the linker untouched for now.

The second commit fixes another runtime error without --emit=llvm-bc/llvm-ir. (my understanding is that we should not have to set these after this PR, because no device.bc anymore)
The “already wrapped” check in handle_offload relied on a value name, which LLVM drops unless --emit=llvm-bc/llvm-ir is given, so the kernel got wrapped twice.

I verified on UofT nvidia gpunode (amdgpunodes was not available for my account).

r? ZuseZ4

[ZuseZ4]

Thank you for also solving the fixme along the way. I'm at the llvm dev till wednesday, but I'll try to test it a bit in between.

@kjetilkjeka you had some ideas on how to configure cpu vs gpu builds, right?

fyi @xkevio

[kjetilkjeka]

For my cargo experiment with offload support this is a step in the right direction, with a little regression.

It fits well within the model of building a single "cdylib" (ptx/elf) for the device compilation per binary (as opposed to one per crate).

The unfortunate regression is of course that a crate with a single main.rs binary requires the mentioned workaround. I believe the solution is to tell rustc that when -Zoffload=Device is set then what would have been executables on the host side should be treated as cdylib on the device side. This is true for both normal executables and when building tests, for the root host unit we basically want a corresponding cdylib on the device side.

I checked out your PR and added a mapping in collect_crate_types from CrateType::Executable to CrateType::Cdylib and that made my basic examples build and run as expected using nvptx64 and an Nvidia GPU.

I haven't got far enough to test cargo test and don't know if that is within scope for offload right now. If it is there might be something like this that needs to be done for the test harness injection for device as well. We basically want to build a device cdylib built with --cfg test and that kind of things, but obviously without injecting the test harness for the offload device.

[ZuseZ4]

Thanks. I agree that independently of the final design we almost never want to compile the GPU code as a normal binary, even if libc-for-gpu might allow us to do such cursed/funny things. So for now mapping the GPU cratetype seems like the right call, @sgasho can you try to add that? I'd expect that if we remap it early enough it should be unproblematic

[sgasho]

Thanks both. I added the Executable → Cdylib mapping in collect_crate_types,
as kjetilkjeka suggested, since that is where crate types are finalized.
I verified that sample offload works on an nvidia gpunode.

[ZuseZ4]

If a user already requests both the executable and the cdylib, this would end up with two cdylib requests. Can you sort and dedup again?

View changes since the review

[sgasho]

I noticed that we cannot sort + dedup them. It breaks the device build.

error: output of --print=file-names missing when learning about target-specific information from rustc
command was: `/w/333/shotasugano/rust-env/rustup/toolchains/offload-local/bin/rustc - --crate-name ___ --print=file-names -Ctarget-cpu=sm_86 -Zoffload=Device -Zunstable-options --target nvptx64-nvidia-cuda --crate-type bin --crate-type rlib --crate-type dylib --crate-type cdylib --crate-type staticlib --crate-type proc-macro --print=sysroot --print=split-debuginfo --print=crate-name --print=cfg -Wwarnings`

--- stdout
lib___.rlib
lib___.a
___.ptx

It seems that cargo reads the file names in the order of --crate-type bin --crate-type rlib --crate-type dylib --crate-type cdylib --crate-type staticlib --crate-type proc-macro , and sort + dedup broke the correspondence between crate types and file names.

[ZuseZ4]

Thanks, another hundred lines saved. I tested it on an AMD igpu, where it also works.
I left one nit, can you fold that into the commit which introduced the helper? Otherwise lgtm.