Skip to content

Add support for PhysicalStorageBuffer - #237

Draft
jwollen wants to merge 11 commits into
Rust-GPU:mainfrom
jwollen:physical-storage-buffer
Draft

Add support for PhysicalStorageBuffer#237
jwollen wants to merge 11 commits into
Rust-GPU:mainfrom
jwollen:physical-storage-buffer

Conversation

@jwollen

@jwollen jwollen commented Apr 19, 2025

Copy link
Copy Markdown
Contributor

Summary

This adds support for SPV_KHR_physical_storage_buffer and the PhysicalStorageBuffer64 addressing model and expands support for physical pointers.

Includes

Motivation

Allow taking advantage of physical buffer addresses (VK_KHR_buffer_device_address) for more flexible bindless storage buffer access.

While physical pointers are largely optional with increasing support for bindless storage buffers,
fully descriptor-less storage buffers remain very ergonomic, especially in pure compute and ray-tracing shaders.

This allows, for example:

struct Node {
    next: PhysicalPtr<T>,
    payload: f32,
}

#[spirv(fragment)]
fn main(
    #[push_constant] root_node: &PhysicalPtr<Node>,
    out: &mut f32,
) {
    unsafe {
        let mut next_node = *root_node;
        while let Some(node) = next_node.as_ref() {
            *out = *out + node.payload;
            next_node = node.next.as_ref();
        }
    }
}

Implementation steps

  • Allow expressing explicit storage classes
  • Enable PhysicalStorageBuffer64 when PhysicalStorageBufferAddresses is supported
  • Support pointer casts
    • Enable u64 as *mut T and vice versa
    • Defer pointer-cast errors until after storage class inferrnce
  • Aligned
    • Generate Aligned operands for all memory operations
    • Add a SPIR-T pass to strip alignment from non-physical operations
    • Allow memory operands in qptr store/load lowering/lifting
  • Add RestrictedPointer if/where necessary
  • Ensure raw pointer ops are correct or disallowed
  • Add spirv_std utilities for working with physical pointers
    • Add a PhysicalPtr<T> type that wraps physical addresses. /bikeshed
    • Parity with *mut

Prior work

  • [Migrated] OpConvertUToPtr support #119: As noted the qptr route is the most flexible and custom constraints for each instruction don't scale. For example supporting OpBitcast would be hard to model with custom constraints.

Open questions

@LegNeato

Copy link
Copy Markdown
Collaborator

Turns out I am porting a shader that needs this too!

@axelkar

axelkar commented Jun 29, 2025

Copy link
Copy Markdown

While physical pointers are largely optional with increasing support for bindless storage buffers

Are physical pointers more performant than using descriptor indexing? Are they similar to 32-bit handles used for bindless in DirectX? Sorry for asking here, but I haven't found documentation on them.

@jwollen

jwollen commented Jul 1, 2025

Copy link
Copy Markdown
Contributor Author

Turns out I am porting a shader that needs this too!

Will pick it up again soon!

Are physical pointers more performant than using descriptor indexing? Are they similar to 32-bit handles used for bindless in DirectX? Sorry for asking here, but I haven't found documentation on them.

There is no direct equivalent in DX (yet?). Buffer device addresses/physical storage buffer pointers are literally 64-bit pointers to GPU memory. That means, compared to descriptors

  • you can do pointer arithmetics on them
  • you can store them directly in buffers (instead of descriptor indices), saving an indirection into descriptor sets/heaps
  • you therefore don't need to manage descriptors for them
  • they are smaller (storage buffer descriptors are at least 16 bytes in all implementations)

They behave a little different and might take different data paths, so the question performance is not trivial. But in general they should be superior.

@FacelessTiger

Copy link
Copy Markdown

I'd love if this was implemented, one of the few things that are blocking me from transitioning fully to rust GPU. I'm not really willing to sacrifice the ergonomics of pointers for descriptor indexed buffers

@dsvensson

dsvensson commented May 19, 2026

Copy link
Copy Markdown

While waiting for this to be resolved by someone who actually knows something I sent my silicon servant to rebase and hack something up based on the PR description here and the other PR. Just switched my lightmap baker to using it and it seems to work for me. Hopefully @jwollen revisits this and solves it properly. If there's anything of use in my branch, feel free to grab. I didn't write any of it, just steered it with what taste I had to offer without knowing the domain.

main...dsvensson:rust-gpu:physical_storage

The code in the PR description is added pretty much verbatim to an executable example.

#[repr(C)]
#[derive(Copy, Clone, Pod, Zeroable)]
pub struct Node {
    pub next: PhysicalPtr<Node>,
    pub payload: f32,
}

#[spirv(compute(threads(1)))]
pub fn main(
    #[spirv(push_constant)] root_node: &PhysicalPtr<Node>,
    #[spirv(storage_buffer, descriptor_set = 0, binding = 0)] output: &mut f32,
) {
    let mut current = *root_node;
    *output = 0.0;
    unsafe {
        while let Some(node) = current.as_ref() {
            *output += node.payload;
            current = node.next;
        }
    }
}

@dsvensson

dsvensson commented Jul 22, 2026

Copy link
Copy Markdown

Rebased my fork of this PR after f16 and "rm cap checks" by @Firestar99 in main as a new branch: main...dsvensson:rust-gpu:physical_storage_2026_07_22

Been using this feature with great success since May for my above mentioned lightmap baker. Hopefully it gets proper attention from someone who knows what they're doing some day.

The updated example now accumulates f16 node payloads to a f16 storage buffer output at a >4GB offset.

@Firestar99

Copy link
Copy Markdown
Member

Feel free to actually open a PR so we can properly review it, and not have to manually copy in stuff. Some things I've noticed while scrolling through;

  • entirely claude written
  • why are you using unstable f16 and not normal f32?
  • why are those even examples and not compiletests or difftests?
  • why is it touching raytracing? just make a separate PR for that

const HIGH_OFFSET: u64 = 4 * 1024 * 1024 * 1024; // 0x1_0000_0000
what?

//! 2. OpConvertPtrToU to a non-64-bit int on a physical pointer
//! (Vulkan requires 64-bit under PhysicalStorageBuffer64; rustc emits
//! u32 because the target's logical pointer width is 32). Widen to u64
//! and patch the comparison consumer's zero-constant operand.

You know you can just change the pointer width to 64? Also widening random u32 values to u64 can have significant knock-on effects, like can you lift all possible operations to u64? Also what would that do to u32::overflowing_add() -> (u32, bool) and friends that expects a u32 to overflow?

Your RestrictedPhysicalPtr docs say:

/// A physical pointer the caller asserts is non-aliasing. Typed marker
/// today — emitting the SPIR-V `RestrictPointer` decoration end-to-end
/// still needs codegen plumbing (the spec only allows it on memory-object
/// declarations, and the asm!-fed Function local that backs the asm!
/// result is promoted away before linking). Treat the type as a binding
/// contract; misuse is UB once the decoration lands.

So, this type does absolutely nothing today?

@dsvensson

dsvensson commented Jul 25, 2026

Copy link
Copy Markdown

Yep, it's entirely claude mangled of the original PR as I just needed the feature now and never intended to have it upstreamed, instead waiting for the original author to come back. The current state is not mergable, lots of vomit'y slop crap all over the place - potentially breaking stuff I don't use myself. I mentioned this in my earlier comment. I'm ok just having claude rebase it for myself from time to time as it does so effortless and pointing my cargo.toml at a git fork works fine. But as it works so well, and it looked like it had been forgotten in the GH issues, I just wish the feature to be adopted by someone who is invested into the project, such as the original PR author.

The ray_query is not gated correctly which makes RustRover cry. As I'm using this branch in my project, it has to contain any fixes needed for my project to behave correctly. A separate fix ofc.

As for f16 in the example, the example serves as a rebase-verification for me and I experimented with using that f16 type in my lightmap baker, again, tailored to my usecase. Don't see that example being part of any real PR, and it's not a PR, so it's fine.

Not requiring or requesting anything, just hoping with fingers crossed.

@Firestar99

Copy link
Copy Markdown
Member

All good, at first this sounded like you wanting to actually upstream your branch, so I thought I'd have a quick look only to notice the sloppyness

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants