feat(mir): add Drop glue translation for device code#409
Merged
Conversation
honeyspoon
force-pushed
the
add-device-drop-glue
branch
2 times, most recently
from
July 20, 2026 13:31
14f922d to
d0ac741
Compare
Enable types with effectful Drop implementations on device by emitting actual drop_in_place::<T> calls when the no-op proof fails. Changes: - mir-importer (drop_glue.rs): add emit_drop_glue() and compute_drop_place_address() -- when drop_glue_is_noop() fails, emit a MirCallOp to the monomorphized drop_in_place::<T> - mir-importer (terminator/mod.rs): change translate_drop() from error on effectful drop to emit_drop_glue() fallback - rustc-codegen-cuda (collector.rs): discover and collect drop_in_place::<T> functions, process TerminatorKind::Drop to enqueue drop glue instances, allow InstanceKind::DropGlue through the instance filter - Convert error_drop_glue from expected-failure to positive test (drop_glue example with end-to-end verification) - Preserve existing no-op fast path as optimization Signed-off-by: abder <bobmatt911@gmail.com>
honeyspoon
force-pushed
the
add-device-drop-glue
branch
from
July 22, 2026 01:16
d0ac741 to
40e76a4
Compare
The drop-glue emission path shipped with a type-level widening of the no-op proof: a Drop impl whose fields are all trivially droppable was declared unobservable. That reasoning is false. A drop body's observability is independent of its fields; a raw-pointer RAII guard whose drop writes through the pointer is the canonical counterexample, and the drop_glue example is exactly that shape. The fallback made translate_drop lower such drops to a plain branch, so the destructor never ran on device (verified on GPU at the original head: all 256 outputs read 0x0 instead of the expected 0xDEADBEEF sentinel). Remove the fallback in all three coupled places at once: - all_field_drops_are_empty in mir-importer's drop_glue.rs - drop_fields_have_no_drop in collector.rs - the fallback arm in drop_instance_is_noop (device_codegen filter) and unify the three decisions onto one shared predicate, mir_importer::drop_instance_is_noop (the body-level proof). The collector consults it through a scoped rustc_internal::run, so collection, emission, and translation cannot drift: everything the importer calls is collected, and provably no-op shims are skipped at discovery time, which also stops transitively collecting dead stdlib Drop::drop bodies (e.g. IntoIter's PolymorphicIter) whose MIR the device pipeline cannot compile. Also revert the Assert-follows-success-edge widening in body_is_noop: now that a failed proof emits the real drop_in_place call, an assert must fail the proof rather than silently delete a would-be device trap. Verified on GPU: cargo oxide run drop_glue now observes the sentinel in all 256 elements, and cargo oxide run array_for_loop still passes with the IntoIter shims proven no-op at collection time. Signed-off-by: nihalp <nihalp@nvidia.com>
"Effectful drop glue is now fully supported" overclaimed. State precisely what holds: provably no-op glue lowers to a plain branch; otherwise the drop lowers to a device-side drop_in_place call, covering direct impl Drop types reachable from kernels; and drop glue whose MIR uses constructs the pipeline cannot yet translate (slice/Vec element drops, panic formatting) still fails compilation with a diagnostic. Destructors are never silently skipped. Signed-off-by: nihalp <nihalp@nvidia.com>
honeyspoon
force-pushed
the
add-device-drop-glue
branch
from
July 22, 2026 13:28
0857b25 to
bd0c1ce
Compare
nihalpasham
force-pushed
the
add-device-drop-glue
branch
from
July 22, 2026 15:17
bd0c1ce to
0857b25
Compare
Collaborator
|
Thanks for the PR. The makes sense for device RAII, device-side calls to the monomorphized
For example: Reviewer note: mir-importer 803 and backend 17 tests pass; #437 is related but stays open. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Enable types with effectful
Dropimplementations on device by emitting actualdrop_in_place::<T>calls when the no-op proof fails. Preserves the existing no-op fast path as an optimization.Motivation
TerminatorKind::Dropis currently a support gap (tracked aserror_drop_glueinSTATUS.md). Destructors that do observable work (logging, releasing resources, resetting state) fail compilation. This PR closes that gap by falling back to real drop glue when the no-op proof fails.Implementation
drop_glue.rs): addemit_drop_glue()andcompute_drop_place_address()-- whendrop_glue_is_noop()fails, emit aMirCallOpto the monomorphizeddrop_in_place::<T>terminator/mod.rs): changetranslate_drop()fromif noop → branch; else → errortoif noop → branch; else → emit_drop_glue()collector.rs): discover and collectdrop_in_place::<T>functions viaTerminatorKind::Drop, allowInstanceKind::DropGluethrough the instance filtererror_drop_gluefrom expected-failure to positive test (drop_glueexample with end-to-end verification)STATUS.mdandsmoketest.shto removeerror_drop_gluefrom error examplesTesting
Validated on RTX A2000 (sm_86), nightly-2026-04-03: