Rust-GPU / Rust-GPU/rust-gpu

[Migrated] spirv-std inline `asm!` should use `MaybeUninit<T>` instead of `let mut result = T::default();`.

Open
#93 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
3.4k
Forks
125
PR merge metrics
No merged PRs in 30d

Description

Issue automatically imported from old repo: https://github.com/EmbarkStudios/rust-gpu/issues/1007
Old labels: t: enhancement,a: asm
Originally creatd by eddyb on 2023-03-17T20:01:45Z


We have a few different ways to extract results from asm!:

  • old unsound approach (UB because of OpReturnValue, so it's getting replaced)
asm! {
    "%result = ...",
    "OpReturnValue %result",
    options(noreturn),
}
  • old approach (sound, but requires T: Default)
let mut result = T::default();
asm!(
    "%result = ...",
    "OpStore {result} %result",
    result = in(reg) &mut result,
);
result
  • new approach (using MaybeUninit<T>), from #1006
let mut result_slot = core::mem::MaybeUninit::uninit();
asm! {
    "%result = ...",
    "OpStore {result_slot} %result",
    result_slot = in(reg) result_slot.as_mut_ptr(),
}
result_slot.assume_init()

In #1006 only the unsound uses of asm!("OpReturnValue") were fixed (i.e. when dealing with opaque handles or &T/&mut T, neither of which implement Default).

However, it might be a good to transition everything uniformly, and maybe even provide a wrapper macro that allows uniformly using asm! with the result slot automatically handled (and even by-ref inputs as well?).

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Review the existing spirv-std inline asm! result-extraction sites and the MaybeUninit approach from #1006. Determine the scope of replacing T::default() result slots uniformly, including whether a wrapper macro is needed; done means applicable extraction no longer requires T: Default and the chosen approach is consistent.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
compilers
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.