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

Disallow use of OpCompositeExtract/OpCompositeInsert with no indices (#2980)

Browse Source
This commit is contained in:
Jakub Kuderski 2019-10-17 13:53:34 -04:00 committed by GitHub
parent 2ca4fcfdc2
commit e3da3143b2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 53 additions and 182 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
source/opt/folding_rules.cpp
View File

@ -1407,20 +1407,6 @@ FoldingRule CompositeExtractFeedingConstruct() {
};
}
// Folds an OpCompositeExtract instruction with no indexes into an OpCopyObject.
bool ExtractWithNoIndexes(IRContext*, Instruction* inst,
const std::vector<const analysis::Constant*>&) {
assert(inst->opcode() == SpvOpCompositeExtract &&
"Wrong opcode. Should be OpCompositeExtract.");
if (inst->NumInOperands() > 1) {
return false;
}
inst->SetOpcode(SpvOpCopyObject);
return true;
}
FoldingRule InsertFeedingExtract() {
return [](IRContext* context, Instruction* inst,
const std::vector<const analysis::Constant*>&) {
@ -2227,7 +2213,6 @@ void FoldingRules::AddFoldingRules() {
// Take that into consideration.
rules_[SpvOpCompositeConstruct].push_back(CompositeExtractFeedingConstruct());
rules_[SpvOpCompositeExtract].push_back(ExtractWithNoIndexes);
rules_[SpvOpCompositeExtract].push_back(InsertFeedingExtract());
rules_[SpvOpCompositeExtract].push_back(CompositeConstructFeedingExtract());
rules_[SpvOpCompositeExtract].push_back(VectorShuffleFeedingExtract());

20
source/val/validate_composites.cpp
View File

@ -30,8 +30,8 @@ namespace {
// OpCompositeInsert instruction. The function traverses the hierarchy of
// nested data structures (structs, arrays, vectors, matrices) as directed by
// the sequence of indices in the instruction. May return error if traversal
// fails (encountered non-composite, out of bounds, nesting too deep).
// Returns the type of Composite operand if the instruction has no indices.
// fails (encountered non-composite, out of bounds, no indices, nesting too
// deep).
spv_result_t GetExtractInsertValueType(ValidationState_t& _,
const Instruction* inst,
uint32_t* member_type) {
@ -40,10 +40,15 @@ spv_result_t GetExtractInsertValueType(ValidationState_t& _,
uint32_t word_index = opcode == SpvOpCompositeExtract ? 4 : 5;
const uint32_t num_words = static_cast<uint32_t>(inst->words().size());
const uint32_t composite_id_index = word_index - 1;
const uint32_t num_indices = num_words - word_index;
const uint32_t kCompositeExtractInsertMaxNumIndices = 255;
if (num_indices > kCompositeExtractInsertMaxNumIndices) {
if (num_indices == 0) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< "Expected at least one index to Op"
<< spvOpcodeString(inst->opcode()) << ", zero found";
} else if (num_indices > kCompositeExtractInsertMaxNumIndices) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< "The number of indexes in Op" << spvOpcodeString(opcode)
<< " may not exceed " << kCompositeExtractInsertMaxNumIndices
@ -386,20 +391,20 @@ spv_result_t ValidateCompositeExtract(ValidationState_t& _,
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< "Cannot extract from a composite of 8- or 16-bit types";
}
return SPV_SUCCESS;
}
spv_result_t ValidateCompositeInsert(ValidationState_t& _,
const Instruction* inst) {
const SpvOp opcode = inst->opcode();
const uint32_t object_type = _.GetOperandTypeId(inst, 2);
const uint32_t composite_type = _.GetOperandTypeId(inst, 3);
const uint32_t result_type = inst->type_id();
if (result_type != composite_type) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< "The Result Type must be the same as Composite type in Op"
<< spvOpcodeString(opcode) << " yielding Result Id " << result_type
<< ".";
<< spvOpcodeString(inst->opcode()) << " yielding Result Id "
<< result_type << ".";
}
uint32_t member_type = 0;
@ -421,6 +426,7 @@ spv_result_t ValidateCompositeInsert(ValidationState_t& _,
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< "Cannot insert into a composite of 8- or 16-bit types";
}
return SPV_SUCCESS;
}

11
test/opt/fold_test.cpp
View File

@ -3333,16 +3333,7 @@ INSTANTIATE_TEST_SUITE_P(CompositeExtractFoldingTest, GeneralInstructionFoldingT
"%3 = OpCompositeExtract %float %2 4\n" +
"OpReturn\n" +
"OpFunctionEnd",
3, 0),
// Test case 14: Fold OpCompositeExtract with no indexes.
InstructionFoldingCase<uint32_t>(
Header() + "%main = OpFunction %void None %void_func\n" +
"%main_lab = OpLabel\n" +
"%2 = OpConstantComposite %v4float %float_1 %float_1 %float_1 %float_1\n" +
"%3 = OpCompositeExtract %v4float %2\n" +
"OpReturn\n" +
"OpFunctionEnd",
3, 2)
3, 0)
));
INSTANTIATE_TEST_SUITE_P(CompositeConstructFoldingTest, GeneralInstructionFoldingTest,

28
test/opt/reduce_load_size_test.cpp
View File

@ -321,34 +321,6 @@ OpFunctionEnd
SinglePassRunAndCheck<ReduceLoadSize>(test, test, true, false);
}
TEST_F(ReduceLoadSizeTest, extract_with_no_index) {
const std::string test =
R"(
OpCapability ImageGatherExtended
OpExtension ""
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %4 "P<EFBFBD>Ma'" %12 %17
OpExecutionMode %4 OriginUpperLeft
%void = OpTypeVoid
%3 = OpTypeFunction %void
%float = OpTypeFloat 32
%_struct_7 = OpTypeStruct %float %float
%_ptr_Input__struct_7 = OpTypePointer Input %_struct_7
%_ptr_Output__struct_7 = OpTypePointer Output %_struct_7
%12 = OpVariable %_ptr_Input__struct_7 Input
%17 = OpVariable %_ptr_Output__struct_7 Output
%4 = OpFunction %void DontInline|Pure|Const %3
%245 = OpLabel
%13 = OpLoad %_struct_7 %12
%33 = OpCompositeExtract %_struct_7 %13
OpReturn
OpFunctionEnd
)";
auto result = SinglePassRunAndDisassemble<ReduceLoadSize>(test, true, true);
EXPECT_EQ(Pass::Status::SuccessWithoutChange, std::get<1>(result));
}
} // namespace
} // namespace opt
} // namespace spvtools

31
test/opt/scalar_replacement_test.cpp
View File

@ -1899,37 +1899,6 @@ TEST_F(ScalarReplacementTest, RelaxedPrecisionMemberDecoration) {
SinglePassRunAndMatch<ScalarReplacementPass>(text, true);
}
TEST_F(ScalarReplacementTest, ExtractWithNoIndex) {
const std::string text = R"(
; CHECK: [[var1:%\w+]] = OpVariable %_ptr_Function_float Function
; CHECK: [[var2:%\w+]] = OpVariable %_ptr_Function_float Function
; CHECK: [[ld1:%\w+]] = OpLoad %float [[var1]]
; CHECK: [[ld2:%\w+]] = OpLoad %float [[var2]]
; CHECK: [[constr:%\w+]] = OpCompositeConstruct {{%\w+}} [[ld2]] [[ld1]]
; CHECK: OpCompositeExtract {{%\w+}} [[constr]]
OpCapability Shader
OpExtension ""
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %1 "main"
OpExecutionMode %1 OriginUpperLeft
%void = OpTypeVoid
%3 = OpTypeFunction %void
%float = OpTypeFloat 32
%_struct_5 = OpTypeStruct %float %float
%_ptr_Private__struct_5 = OpTypePointer Private %_struct_5
%_ptr_Function__struct_5 = OpTypePointer Function %_struct_5
%1 = OpFunction %void Inline %3
%8 = OpLabel
%9 = OpVariable %_ptr_Function__struct_5 Function
%10 = OpLoad %_struct_5 %9
%11 = OpCompositeExtract %_struct_5 %10
OpReturn
OpFunctionEnd
)";
SinglePassRunAndMatch<ScalarReplacementPass>(text, true);
}
} // namespace
} // namespace opt
} // namespace spvtools

68
test/opt/vector_dce_test.cpp
View File

@ -1158,74 +1158,6 @@ OpFunctionEnd
SinglePassRunAndCheck<VectorDCE>(text, text, true, true);
}
TEST_F(VectorDCETest, InsertWithNoIndices) {
const std::string text = R"(
; CHECK: OpEntryPoint Fragment {{%\w+}} "PSMain" [[in1:%\w+]] [[in2:%\w+]] [[out:%\w+]]
; CHECK: OpFunction
; CHECK: [[ld:%\w+]] = OpLoad %v4float [[in2]]
; CHECK: OpStore [[out]] [[ld]]
OpCapability Shader
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %1 "PSMain" %2 %14 %3
OpExecutionMode %1 OriginUpperLeft
%void = OpTypeVoid
%5 = OpTypeFunction %void
%float = OpTypeFloat 32
%v4float = OpTypeVector %float 4
%_ptr_Input_v4float = OpTypePointer Input %v4float
%_ptr_Output_v4float = OpTypePointer Output %v4float
%2 = OpVariable %_ptr_Input_v4float Input
%14 = OpVariable %_ptr_Input_v4float Input
%3 = OpVariable %_ptr_Output_v4float Output
%1 = OpFunction %void None %5
%10 = OpLabel
%13 = OpLoad %v4float %14
%11 = OpLoad %v4float %2
%12 = OpCompositeInsert %v4float %13 %11
OpStore %3 %12
OpReturn
OpFunctionEnd
)";
SinglePassRunAndMatch<VectorDCE>(text, true);
}
TEST_F(VectorDCETest, ExtractWithNoIndices) {
const std::string text = R"(
; CHECK: OpLoad %float
; CHECK: [[ld:%\w+]] = OpLoad %v4float
; CHECK: [[ex1:%\w+]] = OpCompositeExtract %v4float [[ld]]
; CHECK: [[ex2:%\w+]] = OpCompositeExtract %float [[ex1]] 1
; CHECK: OpStore {{%\w+}} [[ex2]]
OpCapability Shader
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %1 "PSMain" %2 %14 %3
OpExecutionMode %1 OriginUpperLeft
%void = OpTypeVoid
%5 = OpTypeFunction %void
%float = OpTypeFloat 32
%v4float = OpTypeVector %float 4
%_ptr_Input_float = OpTypePointer Input %float
%_ptr_Input_v4float = OpTypePointer Input %v4float
%_ptr_Output_float = OpTypePointer Output %float
%2 = OpVariable %_ptr_Input_v4float Input
%14 = OpVariable %_ptr_Input_float Input
%3 = OpVariable %_ptr_Output_float Output
%1 = OpFunction %void None %5
%10 = OpLabel
%13 = OpLoad %float %14
%11 = OpLoad %v4float %2
%12 = OpCompositeInsert %v4float %13 %11 0
%20 = OpCompositeExtract %v4float %12
%21 = OpCompositeExtract %float %20 1
OpStore %3 %21
OpReturn
OpFunctionEnd
)";
SinglePassRunAndMatch<VectorDCE>(text, true);
}
} // namespace
} // namespace opt
} // namespace spvtools

62
test/val/val_composites_test.cpp
View File

@ -648,7 +648,6 @@ TEST_F(ValidateComposites, CompositeExtractSuccess) {
%val16 = OpCompositeExtract %f32 %struct 4 1000 1
%val17 = OpCompositeExtract %f32 %struct 5 0
%val18 = OpCompositeExtract %u32 %struct 5 1
%val19 = OpCompositeExtract %big_struct %struct
)";
CompileSuccessfully(GenerateShaderCode(body));
@ -767,6 +766,18 @@ TEST_F(ValidateComposites, CompositeExtractTooManyIndices) {
"indexes still remain to be traversed."));
}
TEST_F(ValidateComposites, CompositeExtractNoIndices) {
const std::string body = R"(
%struct = OpLoad %big_struct %var_big_struct
%val1 = OpCompositeExtract %big_struct %struct
)";
CompileSuccessfully(GenerateShaderCode(body));
ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("Expected at least one index to OpCompositeExtract"));
}
TEST_F(ValidateComposites, CompositeExtractWrongType1) {
const std::string body = R"(
%struct = OpLoad %big_struct %var_big_struct
@ -861,7 +872,6 @@ TEST_F(ValidateComposites, CompositeInsertSuccess) {
%val16 = OpCompositeInsert %big_struct %f32_3 %struct 4 1000 1
%val17 = OpCompositeInsert %big_struct %f32_3 %struct 5 0
%val18 = OpCompositeInsert %big_struct %u32_3 %struct 5 1
%val19 = OpCompositeInsert %big_struct %struct %struct
)";
CompileSuccessfully(GenerateShaderCode(body));
@ -1157,9 +1167,8 @@ TEST_F(ValidateComposites, CompositeInsertWrongResultTypeBad) {
HasSubstr("The Result Type must be the same as Composite type"));
}
// Valid: No Indexes were passed to OpCompositeExtract, and the Result Type is
// the same as the Base Composite type.
TEST_F(ValidateComposites, CompositeExtractNoIndexesGood) {
// Invalid: No Indexes were passed to OpCompositeExtract.
TEST_F(ValidateComposites, CompositeExtractNoIndices2) {
std::ostringstream spirv;
spirv << GetHeaderForTestsFromValId() << std::endl;
spirv << "%matrix = OpLoad %mat4x3 %my_matrix" << std::endl;
@ -1167,29 +1176,32 @@ TEST_F(ValidateComposites, CompositeExtractNoIndexesGood) {
spirv << R"(OpReturn
OpFunctionEnd)";
CompileSuccessfully(spirv.str());
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
EXPECT_THAT(
getDiagnosticString(),
HasSubstr(
"Expected at least one index to OpCompositeExtract, zero found"));
}
// Invalid: No Indexes were passed to OpCompositeExtract, but the Result Type is
// different from the Base Composite type.
TEST_F(ValidateComposites, CompositeExtractNoIndexesBad) {
// Invalid: No Indexes were passed to OpCompositeExtract.
TEST_F(ValidateComposites, CompositeExtractNoIndicesWrongResultType) {
std::ostringstream spirv;
spirv << GetHeaderForTestsFromValId() << std::endl;
spirv << "%matrix = OpLoad %mat4x3 %my_matrix" << std::endl;
spirv << "%float_entry = OpCompositeExtract %float %matrix" << std::endl;
spirv << "%float_entry = OpCompositeExtract %float %matrix" << std::endl;
spirv << R"(OpReturn
OpFunctionEnd)";
CompileSuccessfully(spirv.str());
EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("Result type (OpTypeFloat) does not match the type "
"that results from indexing into the composite "
"(OpTypeMatrix)."));
EXPECT_THAT(
getDiagnosticString(),
HasSubstr(
"Expected at least one index to OpCompositeExtract, zero found"));
}
// Valid: No Indexes were passed to OpCompositeInsert, and the type of the
// Invalid: No Indices were passed to OpCompositeInsert, and the type of the
// Object<id> argument matches the Composite type.
TEST_F(ValidateComposites, CompositeInsertMissingIndexesGood) {
TEST_F(ValidateComposites, CompositeInsertMissingIndices) {
std::ostringstream spirv;
spirv << GetHeaderForTestsFromValId() << std::endl;
spirv << "%matrix = OpLoad %mat4x3 %my_matrix" << std::endl;
@ -1199,12 +1211,16 @@ TEST_F(ValidateComposites, CompositeInsertMissingIndexesGood) {
OpReturn
OpFunctionEnd)";
CompileSuccessfully(spirv.str());
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
EXPECT_THAT(
getDiagnosticString(),
HasSubstr(
"Expected at least one index to OpCompositeInsert, zero found"));
}
// Invalid: No Indexes were passed to OpCompositeInsert, but the type of the
// Invalid: No Indices were passed to OpCompositeInsert, but the type of the
// Object<id> argument does not match the Composite type.
TEST_F(ValidateComposites, CompositeInsertMissingIndexesBad) {
TEST_F(ValidateComposites, CompositeInsertMissingIndices2) {
std::ostringstream spirv;
spirv << GetHeaderForTestsFromValId() << std::endl;
spirv << "%matrix = OpLoad %mat4x3 %my_matrix" << std::endl;
@ -1214,10 +1230,10 @@ TEST_F(ValidateComposites, CompositeInsertMissingIndexesBad) {
OpFunctionEnd)";
CompileSuccessfully(spirv.str());
EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("The Object type (OpTypeInt) does not match the type "
"that results from indexing into the Composite "
"(OpTypeMatrix)."));
EXPECT_THAT(
getDiagnosticString(),
HasSubstr(
"Expected at least one index to OpCompositeInsert, zero found"));
}
// Valid: Tests that we can index into Struct, Array, Matrix, and Vector!