Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[0010] Address writability of vk::BufferPointer<T,A> pointees #41

Closed
llvm-beanz opened this issue Apr 14, 2023 · 7 comments · Fixed by #52
Closed

[0010] Address writability of vk::BufferPointer<T,A> pointees #41

llvm-beanz opened this issue Apr 14, 2023 · 7 comments · Fixed by #52
Labels
active proposal Issues relating to active proposals

Comments

@llvm-beanz
Copy link
Collaborator

Which proposal does this relate to?
0010-vk-buffer-ref.md

Describe the issue or outstanding question.
The spec mentions vk::RawBufferStore, but doesn't explicitly clarify if or how writing through a vk::BufferPointer<T,A> is supported. We should clarify this case.

If writing through vk::BufferPointer<T,A> is allowed we also need to spec how this works with HLSL's copy-in/copy-out paramter passing as well as pointer address spacing.

Additional context
See initial review discussion.

@llvm-beanz llvm-beanz added the active proposal Issues relating to active proposals label Apr 14, 2023
@greg-lunarg
Copy link
Contributor

The client I am proposing this for is only interested in reading the pointee. But I am guessing there is interest in writing as well. Still, it would be good to have at least one real live potential buffer pointer user testify that they want to write. @natevm, do you want to be able to write to the pointee?

@natevm
Copy link

natevm commented Apr 14, 2023

The client I am proposing this for is only interested in reading the pointee. But I am guessing there is interest in writing as well. Still, it would be good to have at least one real live potential buffer pointer user testify that they want to write. @natevm, do you want to be able to write to the pointee?

Yeah, my end users at Argonne labs both read and write to pointers throughout all our Vulkan accelerated projects.

@llvm-beanz
Copy link
Collaborator Author

The client I am proposing this for is only interested in reading the pointee.

While I appreciate that you have your priorities and that your client probably ranks up there... We should consider the applications of the feature and how they might be useful to a wider audience.

@greg-lunarg
Copy link
Contributor

We should consider the applications of the feature and how they might be useful to a wider audience.

My point was only that I couldn't recall seeing a request to write to a pointee. I always like to make sure I have at least one customer before I create a capability :)

@devshgraphicsprogramming

I'd definitely be interested in the optimizer optimizing away stores on variables which have not changed, i.e.

struct Joint
{
   float3x4 relativeTForm;
   vk::BufferPointer<Joint> parent;
   uint modifiedStamp;
   uint recomputedStamp;
   float3x4 absoluteTForm;
};

...

Joint tmp = ptr.get();
tmp.absoluteTForm = special_mul(tmp.parent.get().absoluteTForm,tmp.relativeTForm);
tmp.recomputedStamp = tmp.modifiedStamp;
ptr.get() = tmp;

I'd expect this to only store the modified fields after inlining and optimization.

@llvm-beanz
Copy link
Collaborator Author

Prefacing with: We should not specify optimization behavior in the spec. The spec needs to document the language behavior, optimizations need to be spec-conforming and are implementation defined.

I would expect optimizers to remove stores to non-written fields if it is safe to do so, but given the example above, optimizing stores may not be safe depending on memory access and aliasing rules (which we haven’t yet fully defined).

If a vk::BufferPointer<T> is assumed to be non-aliasing and writes are unordered, we could optimize away writes of non-modified fields, and even coalesce writes where the same field might be written to more than once with different values. I would expect existing LLVM optimizations (SROA, dead store elimination, etc) to perform that transformation (no idea if SPIR-V-opt will).

Since vk::BuffferPointer<T>::Get is defined returning an lvalue reference, I would expect that the call is fully optimizable down to just an address being returned and does not imply a load by itself. Instead I would expect member assignment or access of the return value to constitute the load.

In HLSL 202x with user-defined references, you could write the code instead to write back to the vk::BufferPointer<T> by storing a local reference. In that case, I would expect the writes to only write the modified field regardless of aliasing restrictions. That wouldn’t be an optimization but the specified behavior.

@greg-lunarg
Copy link
Contributor

Given the response from @natevm et al, I will add some language to the proposal that writes of pointees is supported.

greg-lunarg added a commit to greg-lunarg/hlsl-specs that referenced this issue May 5, 2023
llvm-beanz pushed a commit that referenced this issue Sep 1, 2023
* Add writability to vk::BufferPointer pointees

Fixes #41

* Add example of write through vk::BufferPointer

* Add SPIR-V for write through buffer pointer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
active proposal Issues relating to active proposals
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants