Skip to content

Conversation

eddyb
Copy link
Collaborator

@eddyb eddyb commented Aug 10, 2025

This is mainly a reshuffling of terminology and organization, though an important one IMO.

The rough split is along the lines of:

  • "memory" as a storage medium for data (and more exotic "handles" etc.)
    • ("data" as a term for e.g. Rust [MaybeUninit<u8>] - or [Byte] given this Byte definition)
    • MemOp is mainly accesses like Load and Store (also Copy, maybe atomics, in the future)
    • mem::layout/mem::shape could use more reorganization, but are a better fit than qptr
      (because they describe the contents of memory, e.g. as viewed through some data type)
    • "typed memory" is produced by mem::analyze collecting MemAccesses into #[mem.accesses(...)] attributes, which can be turned into data types (and typed pointers in e.g. qptr::lift)
    • "HAPP" ("Hierarchical Access Pattern Partitioning") is an acronym I am not proud of, but it's quite descriptive when it comes to the nested-data-type-shaped access patterns that mem::analyze collects (and there is potential for reusing the struct-like "disjoint partitioning")
  • "pointers" as values that can indirectly reference memory ("cursors", in a sense)
    • in theory, different kinds of pointer types can share the same memory infrastructure
      (or not even be needed, e.g. memory accesses could take a memory declaration and an offset)
    • for now, still only qptr and SPIR-V OpTypePointer (maybe a ptr module in the future?)
    • a future qptr::legalize pass is going to rewrite dynamic pointers (what SPIR-V calls "variable pointers") into e.g. pairs of integers (which memory declaration, and what offset), and that is purely about the pointer values (as opposed to the memory they refer to)

The motivation for this refactor was a combination of:

  • the qptr::legalize design making it clear that there should be a split between it, and (what is now) mem::analyze - the latter should only have to infer "typed memory" from direct accesses (and never have to worry about dynamic pointer values)
  • even with Vulkan getting SPV_KHR_untyped_pointers, that only simplifies/removes the need for "typed memory", but it does not remove restrictions on pointer values, so qptr::legalize would remain mostly unchanged, even if mem::analyze (and the worst of qptr::lift) could be skipped
  • wanting to support more than just qptr (e.g. the OpenCL model, or even just PhysicalStorageBuffer64), without having to duplicate/complicate the memory access parts
    • in fact, lowering e.g. OpLoad/OpStore to mem.load/mem.store while keeping the SPIR-V OpTypePointer, would even be doable (if limited to scalar/vector types - anything else would still have to be lowered in a separate pass that needs access to mem::layout, but that's less obvious without the disaggregate changes)

@eddyb eddyb requested a review from a team August 10, 2025 13:56
@LegNeato
Copy link
Collaborator

Does this have the infra to support CUDA's model as well?

@LegNeato

This comment was marked as resolved.

@eddyb eddyb marked this pull request as ready for review August 11, 2025 07:57
@eddyb
Copy link
Collaborator Author

eddyb commented Aug 11, 2025

BTW, this is marked as draft.

Just had to rebase, fixed now.

Does this have the infra to support CUDA's model as well?

To be clear, there's no functional changes in this PR, only moves, renames and documentation changes.

However, supporting the OpenCL/CUDA model (or Vulkan w/ PhysicalStorageBuffer64) was one of the reasons to want something more flexible than qptr, esp. now that SPIR-V seems to be going all-in on untyped pointers - and the separate mem would make more sense with various new pointer kinds.

We will likely never get to map Rust (data) pointers to one specific address space, even in the OpenCL/CUDA model, but having real pointers would mean qptr::legalize's job could be much easier (and in most cases it might not need to do a dynamic choice at all, instead mapping one qptr to one native pointer etc.).

Copy link
Collaborator

@LegNeato LegNeato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fairly mechanical 🚀

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.

2 participants