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

Does it make sense for builtins to have Physical addresses ? #291

Open
Hugobros3 opened this issue Oct 18, 2024 · 3 comments
Open

Does it make sense for builtins to have Physical addresses ? #291

Hugobros3 opened this issue Oct 18, 2024 · 3 comments

Comments

@Hugobros3
Copy link

The Physical32 and Physical64 addressing models assert a fixed-sized, canonical representation for all pointers, meant for use in OpenCL and related environments (i.e. LevelZero), but I'm not sure is this is meant to be the case for Input and particularly builtins.

Actually using them like so seems broken, see this bug in IGC for an example of what I mean: intel/intel-graphics-compiler#347

If the Input storage class work the way I understand them to (compiler only accepts loads to global input variables and transforms them to an appropriate platform-specific intrinsic), then the spec should probably walk back the physical addressing wording to only include storage classes that actually describe physical memory.

cc @bashbaug

@bashbaug
Copy link
Collaborator

Thinking out loud: assuming this is true:

If the Input storage class work the way I understand them to (compiler only accepts loads to global input variables and transforms them to an appropriate platform-specific intrinsic), then the spec should probably walk back the physical addressing wording to only include storage classes that actually describe physical memory.

Maybe we need to add restrictions that pointers to the input storage class can only be used as operands to OpLoad? This restriction could go in the OpenCL SPIR-V environment spec, assuming it cannot go in the core SPIR-V spec.

Note that similar restrictions are rare but do occur in other places: for example, for the pointer returned by OpImageTexelPointer, "use of such a pointer is limited to atomic operations."

@Naghasan
Copy link
Member

answering out loud

Maybe we need to add restrictions that pointers to the input storage class can only be used as operands to OpLoad?

That's probably an ok resolution for the OpenCL/SYCL use case as I suspect 3 elements vectors are the most complex data type that will be used for Input. We could have something more fine grain (e.g. support a bitcast + gep pattern) but I doubt there is an actual need for it and, looking through the lens of the highlighted bug, it is probably only going to create new issues.

This restriction could go in the OpenCL SPIR-V environment spec, assuming it cannot go in the core SPIR-V spec.

I think the environment spec is more appropriate given its specificity.

@Hugobros3
Copy link
Author

Given other APIs use Input variables in a very similar way, I think this is a good candidate item or alignment between them.

I'm also not sure whether it's valid to pass a builtin via OpFunctionParameter in Vulkan/GL

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

No branches or pull requests

3 participants