AuroraMiddleware/SPIRV-Tools
1
0
Fork 0
mirror of https://github.com/KhronosGroup/SPIRV-Tools synced 2024-10-18 19:20:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

Checks for variable pointers (#1976)

Browse Source 
In logical addressing mode, we are not allowed to generate variables
pointers.  There is already a check for OpSelect.  However, OpPhi 
and OpPtrAccessChain are not checked to make sure it does not 
generate an variable pointer.  I've added those checks.

Fixes #1957.
This commit is contained in:
Steven Perron 2018-10-16 14:57:55 -04:00 committed by GitHub
parent ab45d69154
commit b407163ef3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 201 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
source/val/validate_cfg.cpp
View File

@ -48,6 +48,20 @@ spv_result_t ValidatePhi(ValidationState_t& _, const Instruction* inst) {
"basic blocks.";
}
const Instruction* type_inst = _.FindDef(inst->type_id());
assert(type_inst);
const SpvOp type_opcode = type_inst->opcode();
if (type_opcode == SpvOpTypePointer &&
_.addressing_model() == SpvAddressingModelLogical) {
if (!_.features().variable_pointers &&
!_.features().variable_pointers_storage_buffer) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< "Using pointers with OpPhi requires capability "
<< "VariablePointers or VariablePointersStorageBuffer";
}
}
// Create a uniqued vector of predecessor ids for comparison against
// incoming values. OpBranchConditional %cond %label %label produces two
// predecessors in the CFG.

17
source/val/validate_memory.cpp
View File

@ -735,6 +735,19 @@ spv_result_t ValidateAccessChain(ValidationState_t& _,
return SPV_SUCCESS;
}
spv_result_t ValidatePtrAccessChain(ValidationState_t& _,
const Instruction* inst) {
if (_.addressing_model() == SpvAddressingModelLogical) {
if (!_.features().variable_pointers &&
!_.features().variable_pointers_storage_buffer) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< "Generating variable pointers requires capability "
<< "VariablePointers or VariablePointersStorageBuffer";
}
}
return ValidateAccessChain(_, inst);
}
} // namespace
spv_result_t MemoryPass(ValidationState_t& _, const Instruction* inst) {
@ -752,9 +765,11 @@ spv_result_t MemoryPass(ValidationState_t& _, const Instruction* inst) {
case SpvOpCopyMemorySized:
if (auto error = ValidateCopyMemory(_, inst)) return error;
break;
case SpvOpPtrAccessChain:
if (auto error = ValidatePtrAccessChain(_, inst)) return error;
break;
case SpvOpAccessChain:
case SpvOpInBoundsAccessChain:
case SpvOpPtrAccessChain:
case SpvOpInBoundsPtrAccessChain:
if (auto error = ValidateAccessChain(_, inst)) return error;
break;

129
test/val/val_cfg_test.cpp
View File

@ -1929,6 +1929,135 @@ OpFunctionEnd
ASSERT_EQ(SPV_SUCCESS, ValidateInstructions());
}
TEST_F(ValidateCFG, ShaderWithPhiPtr) {
const std::string text = R"(
OpCapability Shader
OpMemoryModel Logical GLSL450
OpEntryPoint GLCompute %1 "main"
OpExecutionMode %1 LocalSize 1 1 1
OpSource HLSL 600
%bool = OpTypeBool
%_ptr_Function_bool = OpTypePointer Function %bool
%void = OpTypeVoid
%5 = OpTypeFunction %void
%1 = OpFunction %void None %5
%6 = OpLabel
%7 = OpVariable %_ptr_Function_bool Function
%8 = OpVariable %_ptr_Function_bool Function
%9 = OpUndef %bool
OpSelectionMerge %10 None
OpBranchConditional %9 %11 %10
%11 = OpLabel
OpBranch %10
%10 = OpLabel
%12 = OpPhi %_ptr_Function_bool %7 %6 %8 %11
OpReturn
OpFunctionEnd
)";
CompileSuccessfully(text);
EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("Using pointers with OpPhi requires capability "
"VariablePointers or VariablePointersStorageBuffer"));
}
TEST_F(ValidateCFG, VarPtrShaderWithPhiPtr) {
const std::string text = R"(
OpCapability Shader
OpCapability VariablePointers
OpExtension "SPV_KHR_variable_pointers"
OpMemoryModel Logical GLSL450
OpEntryPoint GLCompute %1 "main"
OpExecutionMode %1 LocalSize 1 1 1
OpSource HLSL 600
%bool = OpTypeBool
%_ptr_Function_bool = OpTypePointer Function %bool
%void = OpTypeVoid
%5 = OpTypeFunction %void
%1 = OpFunction %void None %5
%6 = OpLabel
%7 = OpVariable %_ptr_Function_bool Function
%8 = OpVariable %_ptr_Function_bool Function
%9 = OpUndef %bool
OpSelectionMerge %10 None
OpBranchConditional %9 %11 %10
%11 = OpLabel
OpBranch %10
%10 = OpLabel
%12 = OpPhi %_ptr_Function_bool %7 %6 %8 %11
OpReturn
OpFunctionEnd
)";
CompileSuccessfully(text);
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
}
TEST_F(ValidateCFG, VarPtrStgBufShaderWithPhiStgBufPtr) {
const std::string text = R"(
OpCapability Shader
OpCapability VariablePointersStorageBuffer
OpExtension "SPV_KHR_variable_pointers"
OpMemoryModel Logical GLSL450
OpEntryPoint GLCompute %1 "main"
OpExecutionMode %1 LocalSize 1 1 1
OpSource HLSL 600
%bool = OpTypeBool
%float = OpTypeFloat 32
%_ptr_StorageBuffer_float = OpTypePointer StorageBuffer %float
%7 = OpVariable %_ptr_StorageBuffer_float StorageBuffer
%8 = OpVariable %_ptr_StorageBuffer_float StorageBuffer
%void = OpTypeVoid
%5 = OpTypeFunction %void
%1 = OpFunction %void None %5
%6 = OpLabel
%9 = OpUndef %bool
OpSelectionMerge %10 None
OpBranchConditional %9 %11 %10
%11 = OpLabel
OpBranch %10
%10 = OpLabel
%12 = OpPhi %_ptr_StorageBuffer_float %7 %6 %8 %11
OpReturn
OpFunctionEnd
)";
CompileSuccessfully(text);
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
}
TEST_F(ValidateCFG, KernelWithPhiPtr) {
const std::string text = R"(
OpCapability Kernel
OpCapability Addresses
OpMemoryModel Physical32 OpenCL
OpEntryPoint Kernel %1 "main"
OpExecutionMode %1 LocalSize 1 1 1
OpSource HLSL 600
%bool = OpTypeBool
%_ptr_Function_bool = OpTypePointer Function %bool
%void = OpTypeVoid
%5 = OpTypeFunction %void
%1 = OpFunction %void None %5
%6 = OpLabel
%7 = OpVariable %_ptr_Function_bool Function
%8 = OpVariable %_ptr_Function_bool Function
%9 = OpUndef %bool
OpSelectionMerge %10 None
OpBranchConditional %9 %11 %10
%11 = OpLabel
OpBranch %10
%10 = OpLabel
%12 = OpPhi %_ptr_Function_bool %7 %6 %8 %11
OpReturn
OpFunctionEnd
)";
CompileSuccessfully(text);
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
}
/// TODO(umar): Nested CFG constructs
} // namespace

49
test/val/val_id_test.cpp
View File

@ -51,7 +51,13 @@ std::string kOpCapabilitySetup = R"(
OpCapability Vector16
)";
std::string kGLSL450MemoryModel = kOpCapabilitySetup + R"(
std::string kOpVariablePtrSetUp = R"(
OpCapability VariablePointers
OpExtension "SPV_KHR_variable_pointers"
)";
std::string kGLSL450MemoryModel =
kOpCapabilitySetup + kOpVariablePtrSetUp + R"(
OpMemoryModel Logical GLSL450
)";
@ -2046,9 +2052,9 @@ TEST_F(ValidateIdWithMessage, OpLoadVarPtrOpPhiGood) {
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
}
// Without the VariablePointers Capability, OpLoad will not allow loading
// through a variable pointer.
TEST_F(ValidateIdWithMessage, OpLoadVarPtrOpPhiBad) {
// Without the VariablePointers Capability, OpPhi can have a pointer result
// type.
TEST_F(ValidateIdWithMessage, OpPhiBad) {
std::string result_strategy = R"(
%is_neg = OpSLessThan %bool %i %zero
OpSelectionMerge %end_label None
@ -2067,8 +2073,10 @@ TEST_F(ValidateIdWithMessage, OpLoadVarPtrOpPhiBad) {
false /* Add VariablePointers Capability?*/,
false /* Use Helper Function? */);
CompileSuccessfully(spirv.str());
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(), HasSubstr("is not a logical pointer"));
EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("Using pointers with OpPhi requires capability "
"VariablePointers or VariablePointersStorageBuffer"));
}
// With the VariablePointer Capability, OpLoad should allow loading through a
@ -4827,6 +4835,33 @@ TEST_F(ValidateIdWithMessage, OpPtrAccessChainGood) {
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
}
TEST_F(ValidateIdWithMessage, StgBufOpPtrAccessChainGood) {
std::string spirv = R"(
OpCapability Shader
OpCapability Linkage
OpCapability VariablePointersStorageBuffer
OpExtension "SPV_KHR_variable_pointers"
OpMemoryModel Logical GLSL450
OpEntryPoint GLCompute %3 ""
%int = OpTypeInt 32 0
%int_2 = OpConstant %int 2
%int_4 = OpConstant %int 4
%struct = OpTypeStruct %int
%array = OpTypeArray %struct %int_4
%ptr = OpTypePointer StorageBuffer %array
%var = OpVariable %ptr StorageBuffer
%1 = OpTypeVoid
%2 = OpTypeFunction %1
%3 = OpFunction %1 None %2
%4 = OpLabel
%5 = OpPtrAccessChain %ptr %var %int_2
OpReturn
OpFunctionEnd
)";
CompileSuccessfully(spirv.c_str());
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
}
TEST_F(ValidateIdWithMessage, OpLoadBitcastPointerGood) {
std::string spirv = kOpenCLMemoryModel64 + R"(
%2 = OpTypeVoid
@ -5039,7 +5074,7 @@ TEST_F(ValidateIdWithMessage, CorrectErrorForShuffle) {
HasSubstr(
"Component index 4 is out of bounds for combined (Vector1 + Vector2) "
"size of 4."));
EXPECT_EQ(23, getErrorPosition().index);
EXPECT_EQ(25, getErrorPosition().index);
}
TEST_F(ValidateIdWithMessage, VoidStructMember) {