[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]

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.

[rust-bors]

☔ The latest upstream changes made this pull request unmergeable. Please resolve the merge conflicts by rebasing.

[ZuseZ4]

@bors delegate r=me when rebased

[rust-bors]

Invalid command: Key-value argument r is not supported. Run @bors help or go to https://bors.rust-lang.org/help to see available commands.

[ZuseZ4]

@bors delegate lgtm, after rebase

[rust-bors]

Unknown argument "lgtm,". Did you mean to use @bors tryorreview``? Run @bors help or go to https://bors.rust-lang.org/help to see available commands.

[ZuseZ4]

I thought bors now allows comments following the command? But I guess that's just for r+. lgtm, after rebase

@bors delegate

[rust-bors]

✌️ @sgasho, you can now approve this pull request!

If @ZuseZ4 told you to "r=me" after making some further change, then please make that change and post @bors r=ZuseZ4.

View changes since this delegation.