Skip to content

[NNNN] DynamicBufferObjects (FromAddress) proposal #579

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mapodaca-nv
Copy link

Dynamic Buffer Objects introduces the ability to create buffer objects directly from GPUVAs.

Copy link
Collaborator

@llvm-beanz llvm-beanz left a comment

Choose a reason for hiding this comment

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

I have two high-level thoughts on this, which are probably worth exploring.

Should this just be pointers?

If we just had pointers in HLSL would we be considering this at all? If this feature is just working around not having pointers, I'd rather we figure out how to add pointers or references.

Memory safety

I don't love that we have no bounds checking on these. It should be pretty straightforward to add a size value to the new functions and support bounds checking. We've previously discussed not adding raw pointers to HLSL and instead adding bounds-checked "range" constructs specifically to avoid introducing memory safety issues.

Similar to conversations that we've had in the past about bounds checking on resources, I think that if we do add strictly unsafe memory operations we will need to think about how to make it opt-in, and something captured in the shader's feature flags so that an application that accepts untrusted inputs (like a web browser) can set a flag on PSO creation to disallow the feature.

@mapodaca-nv
Copy link
Author

Should this just be pointers?

If we just had pointers in HLSL would we be considering this at all? If this feature is just working around not having pointers, I'd rather we figure out how to add pointers or references.

I see your point but there are some advantages to this model. Once the objects are created from the GPUVA, they can be passed to existing, legacy shader code without requiring any changes. And we maintain the strong typing, alignments, and indexing syntax. Even once we have full support for raw pointers, casting those pointers to an object type like this can provide some benefits.

Memory safety

I don't love that we have no bounds checking on these. It should be pretty straightforward to add a size value to the new functions and support bounds checking. We've previously discussed not adding raw pointers to HLSL and instead adding bounds-checked "range" constructs specifically to avoid introducing memory safety issues.

FWIW, this concern seems to conflict with the first desire to add raw pointers. Furthermore, adding a length parameter does nothing more than force the implementation to implement manual bounds checking (something the shader code can implement itself, if desired) on a value that has no more guarantees of being valid than the GPUVA. i.e. If we cannot trust the any part of the shader code then we must not trust all the shader code.

Similar to conversations that we've had in the past about bounds checking on resources, I think that if we do add strictly unsafe memory operations we will need to think about how to make it opt-in, and something captured in the shader's feature flags so that an application that accepts untrusted inputs (like a web browser) can set a flag on PSO creation to disallow the feature.

There is a flag to detect if this feature is being used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants