Skip to content

mir-importer: require closure receiver for rust-call lowering - #214

Merged
nihalpasham merged 2 commits into
NVIDIA:mainfrom
niklebedenko:upstream-closure-receiver-guard
Jun 16, 2026
Merged

nihalpasham merged 2 commits into
NVIDIA:mainfrom
niklebedenko:upstream-closure-receiver-guard

Conversation

@niklebedenko

Copy link
Copy Markdown
Contributor

Closes #213.

Problem

translate_call special-cases closure trait calls because rust-call ABI passes arguments as a tuple while the closure body expects unpacked arguments. The old predicate looked for names containing call_once, call_mut, or ending in ::call, and then checked whether any generic substitution text contained Closure.

That is too broad. A normal wrapper method named call can have a closure as a generic parameter while still expecting its tuple argument as one ordinary method argument. On upstream/main with only the regression applied, ClosureWrapper<F>::call(&self, (u32,)) is misclassified and LLVM verification fails:

Function call has incorrect type:
expected llvm.func <builtin.integer i32(llvm.ptr (0), llvm.struct <{ builtin.integer i32 }>) variadic = false>,
got llvm.func <builtin.integer i32(llvm.ptr (0), builtin.integer i32) variadic = false>

Fix

Require the rust-call receiver to actually be a closure before using translate_closure_call.

The new receiver_is_closure helper inspects the first call operand, resolves its MIR local or constant type, unwraps references, and matches RigidTy::Closure. If the receiver is an ordinary ADT such as ClosureWrapper<F>, the call stays on the normal call-lowering path and keeps its tuple argument intact.

Diligence

I checked the sibling closure-call paths:

  • translate_closure_call still handles the genuine closure trait ABI path, including by-value call_once and by-reference call / call_mut receivers.
  • extract_closure_body_name already requires the receiver operand to be a closure or reference-to-closure before resolving a closure body.
  • the collector has a name-based closure discovery heuristic, but it only adds closure bodies to the collection when a type argument is itself a closure; it does not rewrite call operands or unpack tuples.

The initial wrapper regression used #[inline(always)], which let rustc optimize the wrapper method away before import. The committed regression uses #[inline(never)] so the importer must classify the ordinary wrapper call.

Tests

Extended crates/rustc-codegen-cuda/examples/device_closures with:

  • ClosureWrapper<F>, where F: Fn(u32) -> u32;
  • an out-of-line ClosureWrapper::call(&self, (u32,)) -> u32;
  • test_wrapper_method_named_call, which stores a captured closure in the wrapper and calls the wrapper method from a kernel.

The existing device closure tests still exercise genuine closure calls, including closure trait calls through FnOnce.

Validation

Verified the regression fails on upstream/main by applying only the example change and running:

CUDA_OXIDE_SHOW_RUSTC_MIR=1 cargo oxide run device_closures

Verified the patched branch with:

cargo fmt --all -- --check
cargo check -p mir-importer
cargo clippy -p mir-importer -- -D warnings
(cd crates/rustc-codegen-cuda/examples/device_closures && cargo fmt --all -- --check)
(cd crates/rustc-codegen-cuda/examples/device_closures && cargo clippy --all-targets -- -D warnings)
(cd crates/rustc-codegen-cuda && cargo fmt --all -- --check)
(cd crates/rustc-codegen-cuda && cargo clippy --all-targets -- -D warnings)
scripts/smoketest.sh --compile-only --no-color --only '^device_closures$'
cargo oxide run device_closures
scripts/smoketest.sh --no-color --only '^device_closures$'

@niklebedenko

niklebedenko commented Jun 12, 2026 •

Copy link
Copy Markdown
Contributor Author

Hey @nihalpasham, sorry to land a whole batch on you at once today (#181–#214). Quick map to make triage easier:

They should all be independent, merge or close in whatever order suits you, no rush on any. The mir-importer ones touch the same area, so if they conflict on the way in, ping me and I'll be happy to rebase (or ofc feel free to edit the branch yourself if that's easier). I'm hitting these bugs because I'm leaning on the compiler hard in real use on my own (fairly large) project, so I expect I'll keep finding fixes.

Really enjoying putting the compiler through its paces, and glad to shape how I contribute around whatever's easiest for you!

@nihalpasham

Copy link
Copy Markdown
Collaborator

Thanks for these. Just a heads-up. I'll need some time to review these as I am working through a couple of large ticket items.

niklebedenko and others added 2 commits June 16, 2026 11:28
…/double-ref closure coverage

Explain in receiver_is_closure why typing the base local (ignoring
place.projection) and peeling a single reference is sound for the
rust-call receiver: rustc materializes the closure-trait receiver as the
closure by value or one &/&mut borrow in its own temporary, never an
in-place projection of a larger aggregate and never behind multiple
references.

Add two device_closures kernels that lock this in as permanent coverage:
a genuine closure called through a struct field projection
((holder.f)(x)) and one called through a double reference ((**rr)(x)).
Both must still unpack the rust-call tuple; the generated .ll confirms
each inlines the closure body (mul by factor) and the module passes LLVM
verification, which the misclassification bug would have failed.
@nihalpasham
nihalpasham force-pushed the upstream-closure-receiver-guard branch from 4236277 to c82dcfc Compare June 16, 2026 06:07
@nihalpasham nihalpasham added bug Something isn't working miscompile Produces incorrect PTX/IR (wrong generated code) codegen Device code-generation pipeline (Rust MIR to IR to PTX) labels Jun 16, 2026
@nihalpasham

Copy link
Copy Markdown
Collaborator

LGTM.

  • Layered: a comment on why receiver_is_closure may ignore place.projection or peel one ref, plus projection-receiver and double-ref closure kernels (both still unpack the tuple).
  • cargo oxide run device_closures --arch sm_90 → all tests pass, incl. the two new ones

@nihalpasham
nihalpasham merged commit 06f7571 into NVIDIA:main Jun 16, 2026
24 checks passed
@niklebedenko
niklebedenko deleted the upstream-closure-receiver-guard branch June 24, 2026 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working codegen Device code-generation pipeline (Rust MIR to IR to PTX) miscompile Produces incorrect PTX/IR (wrong generated code)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Only unpack rust-call tuples for actual closure receivers

2 participants