-
Notifications
You must be signed in to change notification settings - Fork 643
Add validation support for Intel SPIR-V extensions #6232
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
base: main
Are you sure you want to change the base?
Conversation
|
Please sign the CLA. Could you also link to the extension text for constant function pointers and alias scopes? I couldn't see them on the SPIRV-Registry. |
7e8e55e to
35b4fa8
Compare
|
@alan-baker I can't find info on these extensions but I encountered them in the wild. Perhaps @bashbaug can shed some light? |
|
SPV_INTEL_memory_access_aliasing These aren't published on the registry since we still consider them "preview extensions", though if you're encountering them in the wild then we really ought to get them finalized and published on the registry. |
| } | ||
|
|
||
| // Validate that the function operand refers to an OpFunction | ||
| if (function_inst->opcode() != spv::Op::OpFunction) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spec seems unclear to me here (since it neglects to properly describe this operand), but I thought it must be a function pointer. @bashbaug thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bashbaug ping
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the ping, I missed this previously.
I think the code is correct and the function operand should be an OpFunction. This matches what I see generated by SPIR-V LLVM Translator lit tests, e.g. https://github.com/KhronosGroup/SPIRV-LLVM-Translator/blob/main/test/extensions/INTEL/SPV_INTEL_function_pointers/function-pointer.ll :
%3 = OpTypeFunction %uint %uint
...
%_ptr_Function_3 = OpTypePointer Function %3
...
%foo_0 = OpConstantFunctionPointerINTEL %_ptr_Function_3 %foo
%foo = OpFunction %uint DontInline %3
…DeclINTEL in validate_id.cpp and validation_state.cpp
This PR adds validation support for three Intel SPIR-V extensions:
OpConstantFunctionPointerINTEL: Added validation logic and test coverage
ArbitraryPrecisionIntegersINTEL: Added type validation support and tests
OpAliasScopeDeclINTEL/OpAliasScopeListDeclINTEL: Added ID validation support and tests
Fixes #6034