Avoid using make_direct_deprecated() in extern "ptx-kernel" by bjorn3 · Pull Request #133932 · rust-lang/rust

bjorn3 · 2024-12-05T19:54:43Z

This method will be removed in the future as it produces a broken ABI that depends on cg_llvm implementation details. After this PR wasm32-unknown-unknown is the only remaining user of make_direct_deprecated().

Fixes #117271
Blocks #38788

rustbot · 2024-12-05T19:54:51Z

r? @wesleywiser

rustbot has assigned @wesleywiser.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

bjorn3 · 2024-12-05T19:55:05Z

tests/assembly/nvptx-kernel-abi/nvptx-kernel-args-abi-v7.rs


 // CHECK: .visible .entry f_single_u8_arg(
-// CHECK: .param .align 1 .b8 f_single_u8_arg_param_0[1]
+// CHECK: .param .u8 f_single_u8_arg_param_0


I presume this has an identical ABI.

That might be correct. My worry here is that it goes against the "ptx-interoperability" docs.

https://docs.nvidia.com/cuda/ptx-writers-guide-to-interoperability/index.html#aggregates-and-unions

Getting these kind of struct correct (per interop docs) was a big reason I couldn't get it "just right" when attempting to remove the PassMode::Direct

The last commit reverts the ABI for this function back to what it was.

bjorn3 · 2024-12-05T19:57:11Z

tests/assembly/nvptx-kernel-abi/nvptx-kernel-args-abi-v7.rs

 // CHECK: .visible .entry f_u32_slice_arg(
-// CHECK: .param .u64 f_u32_slice_arg_param_0
-// CHECK: .param .u64 f_u32_slice_arg_param_1
+// CHECK: .param .align 8 .b8 f_u32_slice_arg_param_0[16]


We don't guarantee any ABI for wide pointers anyway and if this is breaks the ABI all users can be updated to pass #[repr(C)] struct Slice<'a, T> { ptr: *const (), len: usize, _marker: PhantomData<&'a [T]> } on the host side anyway (and preferably also the gpu side to avoid depending on the unstable ABI).

bjorn3 · 2024-12-05T19:57:44Z

tests/assembly/nvptx-kernel-abi/nvptx-kernel-args-abi-v7.rs

-// CHECK: .visible .entry f_tuple_u8_u8_u32_arg(
-// CHECK: .param .align 4 .b8 f_tuple_u8_u8_u32_arg_param_0[8]
-#[no_mangle]
-pub unsafe extern "ptx-kernel" fn f_tuple_u8_u8_u32_arg(_a: (u8, u8, u32)) {}


Tuples don't have a stable ABI for any calling convention. And can always be replaced with structs anyway.

bjorn3 · 2024-12-05T19:59:25Z

cc @kjetilkjeka @RDambrosio016 as target maintainers, but given that this is a tier 3 target without documentation about what the extern "ptx-kernel" ABI is actually supposed to be, I don't think it is worth blocking on their approval. Having make_direct_deprecated() at all blocks removal broken code in the ABI handling of codegen backends.

kjetilkjeka · 2024-12-06T09:15:10Z

I will try to run this patch through my workplace internal rust CUDA projects and see if anything interesting turns up later today.

vetleras · 2024-12-06T13:32:24Z

I'm a colleague of Kjetil. This change does not seem to break our codebase.

LegNeato · 2025-01-04T02:13:55Z

This method will be removed in the future as it produces a broken ABI that depends on cg_llvm implementation details. After this PR wasm32-unknown-unknown is the only remaining user of make_direct_deprecated().

FYI rust-gpu uses make_direct_deprecated(), as previously mentioned: #115666 (comment)

This method will be removed in the future as it produces a broken ABI that depends on cg_llvm implementation details. After this PR wasm32-unknown-unknown is the only remaining user of make_direct_deprecated().

wesleywiser · 2025-02-05T18:56:11Z

@bors r+ rollup

bors · 2025-02-05T18:56:15Z

📌 Commit 1c1c13a has been approved by wesleywiser

It is now in the queue for this repository.

…kingjubilee Rollup of 13 pull requests Successful merges: - rust-lang#133932 (Avoid using make_direct_deprecated() in extern "ptx-kernel") - rust-lang#136269 (Pass spans around new solver) - rust-lang#136550 (Fix `rustc_hidden_type_of_opaques` for RPITITs with no default body) - rust-lang#136558 (Document minimum supported host tooling on macOS) - rust-lang#136563 (Clean up `Trivial*Impls` macros) - rust-lang#136566 (Fix link in from_fn.rs) - rust-lang#136573 (Document why some "type mismatches" exist) - rust-lang#136583 (Only highlight unmatchable parameters at the definition site) - rust-lang#136587 (Update browser-ui-test version to `0.20.2`) - rust-lang#136590 (Implement RustcInternal for RawPtrKind) - rust-lang#136591 (Add `rustc_hir_pretty::expr_to_string` function) - rust-lang#136595 (Fix `unreachable_pub` lint for hermit target) - rust-lang#136611 (cg_llvm: Remove the `mod llvm_` hack, which should no longer be necessary) Failed merges: - rust-lang#136565 (compiler: Clean up weird `rustc_abi` reexports) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#133932 - bjorn3:fix_ptx_kernel_abi, r=wesleywiser Avoid using make_direct_deprecated() in extern "ptx-kernel" This method will be removed in the future as it produces a broken ABI that depends on cg_llvm implementation details. After this PR wasm32-unknown-unknown is the only remaining user of make_direct_deprecated(). Fixes rust-lang#117271 Blocks rust-lang#38788

…kingjubilee Rollup of 13 pull requests Successful merges: - rust-lang#133932 (Avoid using make_direct_deprecated() in extern "ptx-kernel") - rust-lang#136269 (Pass spans around new solver) - rust-lang#136550 (Fix `rustc_hidden_type_of_opaques` for RPITITs with no default body) - rust-lang#136558 (Document minimum supported host tooling on macOS) - rust-lang#136563 (Clean up `Trivial*Impls` macros) - rust-lang#136566 (Fix link in from_fn.rs) - rust-lang#136573 (Document why some "type mismatches" exist) - rust-lang#136583 (Only highlight unmatchable parameters at the definition site) - rust-lang#136587 (Update browser-ui-test version to `0.20.2`) - rust-lang#136590 (Implement RustcInternal for RawPtrKind) - rust-lang#136591 (Add `rustc_hir_pretty::expr_to_string` function) - rust-lang#136595 (Fix `unreachable_pub` lint for hermit target) - rust-lang#136611 (cg_llvm: Remove the `mod llvm_` hack, which should no longer be necessary) Failed merges: - rust-lang#136565 (compiler: Clean up weird `rustc_abi` reexports) r? `@ghost` `@rustbot` modify labels: rollup

bjorn3 added O-NVPTX Target: the NVPTX LLVM backend for running rust on GPUs, https://llvm.org/docs/NVPTXUsage.html A-ABI Area: Concerning the application binary interface (ABI) labels Dec 5, 2024

rustbot assigned wesleywiser Dec 5, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 5, 2024

bjorn3 commented Dec 5, 2024

View reviewed changes

bjorn3 force-pushed the fix_ptx_kernel_abi branch 2 times, most recently from f38e850 to 9aca298 Compare December 6, 2024 09:11

bjorn3 mentioned this pull request Dec 6, 2024

Remove wasm legacy abi #133952

Merged

kjetilkjeka mentioned this pull request Dec 6, 2024

nvptx "ptx-kernel" ABI (feature: abi_ptx) uses PassMode::Direct for Aggregates #117271

Closed

bjorn3 force-pushed the fix_ptx_kernel_abi branch from 0a44f67 to 7f69c46 Compare December 6, 2024 13:40

bjorn3 added 2 commits January 26, 2025 14:14

Avoid using make_direct_deprecated() in extern "ptx-kernel"

062bc02

This method will be removed in the future as it produces a broken ABI that depends on cg_llvm implementation details. After this PR wasm32-unknown-unknown is the only remaining user of make_direct_deprecated().

Restore previous ABI for f_single_u8_arg

1c1c13a

bjorn3 force-pushed the fix_ptx_kernel_abi branch from 7f69c46 to 1c1c13a Compare January 26, 2025 14:14

wesleywiser approved these changes Feb 5, 2025

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 5, 2025

workingjubilee mentioned this pull request Feb 6, 2025

Rollup of 13 pull requests #136613

Merged

bors merged commit cc0f3ef into rust-lang:master Feb 6, 2025
6 checks passed

rustbot added this to the 1.86.0 milestone Feb 6, 2025

workingjubilee mentioned this pull request Feb 6, 2025

compiler: Clean up weird rustc_abi reexports #136565

Merged

bjorn3 deleted the fix_ptx_kernel_abi branch February 6, 2025 10:01

bjorn3 mentioned this pull request Jun 7, 2025

nvptx abi adjustment may keep PassMode::Direct for small aggregates in conv extern "C" #117480

Closed

Uh oh!

Conversation

bjorn3 commented Dec 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Dec 5, 2024

Uh oh!

bjorn3 Dec 5, 2024

Choose a reason for hiding this comment

Uh oh!

kjetilkjeka Dec 6, 2024

Choose a reason for hiding this comment

Uh oh!

bjorn3 Dec 6, 2024

Choose a reason for hiding this comment

Uh oh!

bjorn3 Dec 5, 2024

Choose a reason for hiding this comment

Uh oh!

bjorn3 Dec 5, 2024

Choose a reason for hiding this comment

Uh oh!

bjorn3 commented Dec 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kjetilkjeka commented Dec 6, 2024

Uh oh!

vetleras commented Dec 6, 2024

Uh oh!

LegNeato commented Jan 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wesleywiser commented Feb 5, 2025

Uh oh!

bors commented Feb 5, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

bjorn3 commented Dec 5, 2024 •

edited

Loading

bjorn3 commented Dec 5, 2024 •

edited

Loading

LegNeato commented Jan 4, 2025 •

edited

Loading