Add validation support for Intel SPIR-V extensions #6232
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. |
…DeclINTEL in validate_id.cpp and validation_state.cpp
pvelesko
force-pushed
the
validation
branch
from
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. |
| spv_result_t ValidateConstantFunctionPointerINTEL(ValidationState_t& _, | ||
| const Instruction* inst) { | ||
| // Check that the FunctionPointersINTEL capability is present | ||
| if (!_.HasCapability(spv::Capability::FunctionPointersINTEL)) { |
There was a problem hiding this comment.
I think this ought to be unnecessary. It should be checked during parsing.
|
|
||
| // Validate the result type is a function pointer | ||
| const auto result_type = _.FindDef(inst->type_id()); | ||
| if (!result_type || result_type->opcode() != spv::Op::OpTypePointer) { |
There was a problem hiding this comment.
Result must be a pointer type. If it is OpTypePointer (i.e. not OpTypeUntypedPointerKHR) then it must match match the one used for the function operand. So basically, this doesn't handle the untyped pointer case.
| // Validate that the function operand exists | ||
| const uint32_t function_id = inst->GetOperandAs<uint32_t>(2); | ||
| const auto function_inst = _.FindDef(function_id); | ||
| if (!function_inst) { |
There was a problem hiding this comment.
Is this actually happening? This indicates there's no instruction for this id. At this point in the flow I don't think that's possible.
| } | ||
|
|
||
| // Validate that the function operand refers to an OpFunction | ||
| if (function_inst->opcode() != spv::Op::OpFunction) { |
There was a problem hiding this comment.
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?
| if (_.HasCapability(spv::Capability::FunctionPointersINTEL)) | ||
| acceptable.push_back(spv::Op::OpConstantFunctionPointerINTEL); |
There was a problem hiding this comment.
Since parsing is gated on the capability already then I think you can just add this to the list unconditionally.
4 participants
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