spirv-fuzz: Avoid void struct member when outlining functions (#3936)
Fixes #3935.
diff --git a/source/fuzz/transformation_outline_function.cpp b/source/fuzz/transformation_outline_function.cpp
index 0eec50e..26f9e0b 100644
--- a/source/fuzz/transformation_outline_function.cpp
+++ b/source/fuzz/transformation_outline_function.cpp
@@ -367,10 +367,10 @@
// Fill out the body of the outlined function according to the region that is
// being outlined.
- PopulateOutlinedFunction(*original_region_entry_block,
- *original_region_exit_block, region_blocks,
- region_output_ids, output_id_to_fresh_id_map,
- ir_context, outlined_function.get());
+ PopulateOutlinedFunction(
+ *original_region_entry_block, *original_region_exit_block, region_blocks,
+ region_output_ids, output_id_to_type_id, output_id_to_fresh_id_map,
+ ir_context, outlined_function.get());
// Collapse the region that has been outlined into a function down to a single
// block that calls said function.
@@ -595,6 +595,12 @@
for (uint32_t output_id : region_output_ids) {
auto output_id_type =
ir_context->get_def_use_mgr()->GetDef(output_id)->type_id();
+ if (ir_context->get_def_use_mgr()->GetDef(output_id_type)->opcode() ==
+ SpvOpTypeVoid) {
+ // We cannot add a void field to a struct. We instead use OpUndef to
+ // handle void output ids.
+ continue;
+ }
struct_member_types.push_back({SPV_OPERAND_TYPE_ID, {output_id_type}});
}
// Add a new struct type to the module.
@@ -758,6 +764,7 @@
const opt::BasicBlock& original_region_exit_block,
const std::set<opt::BasicBlock*>& region_blocks,
const std::vector<uint32_t>& region_output_ids,
+ const std::map<uint32_t, uint32_t>& output_id_to_type_id,
const std::map<uint32_t, uint32_t>& output_id_to_fresh_id_map,
opt::IRContext* ir_context, opt::Function* outlined_function) const {
// When we create the exit block for the outlined region, we use this pointer
@@ -857,12 +864,16 @@
ir_context, SpvOpReturn, 0, 0, opt::Instruction::OperandList()));
} else {
// In the case where there are output ids, we add an OpCompositeConstruct
- // instruction to pack all the output values into a struct, and then an
- // OpReturnValue instruction to return this struct.
+ // instruction to pack all the non-void output values into a struct, and
+ // then an OpReturnValue instruction to return this struct.
opt::Instruction::OperandList struct_member_operands;
for (uint32_t id : region_output_ids) {
- struct_member_operands.push_back(
- {SPV_OPERAND_TYPE_ID, {output_id_to_fresh_id_map.at(id)}});
+ if (ir_context->get_def_use_mgr()
+ ->GetDef(output_id_to_type_id.at(id))
+ ->opcode() != SpvOpTypeVoid) {
+ struct_member_operands.push_back(
+ {SPV_OPERAND_TYPE_ID, {output_id_to_fresh_id_map.at(id)}});
+ }
}
outlined_region_exit_block->AddInstruction(MakeUnique<opt::Instruction>(
ir_context, SpvOpCompositeConstruct,
@@ -948,15 +959,27 @@
// If there are output ids, the function call will return a struct. For each
// output id, we add an extract operation to pull the appropriate struct
- // member out into an output id.
- for (uint32_t index = 0; index < region_output_ids.size(); ++index) {
- uint32_t output_id = region_output_ids[index];
- original_region_entry_block->AddInstruction(MakeUnique<opt::Instruction>(
- ir_context, SpvOpCompositeExtract, output_id_to_type_id.at(output_id),
- output_id,
- opt::Instruction::OperandList(
- {{SPV_OPERAND_TYPE_ID, {message_.new_caller_result_id()}},
- {SPV_OPERAND_TYPE_LITERAL_INTEGER, {index}}})));
+ // member out into an output id. The exception is for output ids with void
+ // type. There are no struct entries for these, so we use an OpUndef of void
+ // type instead.
+ uint32_t struct_member_index = 0;
+ for (uint32_t output_id : region_output_ids) {
+ uint32_t output_type_id = output_id_to_type_id.at(output_id);
+ if (ir_context->get_def_use_mgr()->GetDef(output_type_id)->opcode() ==
+ SpvOpTypeVoid) {
+ original_region_entry_block->AddInstruction(MakeUnique<opt::Instruction>(
+ ir_context, SpvOpUndef, output_type_id, output_id,
+ opt::Instruction::OperandList()));
+ // struct_member_index is not incremented since there was no struct member
+ // associated with this void-typed output id.
+ } else {
+ original_region_entry_block->AddInstruction(MakeUnique<opt::Instruction>(
+ ir_context, SpvOpCompositeExtract, output_type_id, output_id,
+ opt::Instruction::OperandList(
+ {{SPV_OPERAND_TYPE_ID, {message_.new_caller_result_id()}},
+ {SPV_OPERAND_TYPE_LITERAL_INTEGER, {struct_member_index}}})));
+ struct_member_index++;
+ }
}
// Finally, we terminate the block with the merge instruction (if any) that
diff --git a/source/fuzz/transformation_outline_function.h b/source/fuzz/transformation_outline_function.h
index be56695..36c0daf 100644
--- a/source/fuzz/transformation_outline_function.h
+++ b/source/fuzz/transformation_outline_function.h
@@ -178,7 +178,8 @@
// of |original_region_exit_block| so that it returns something appropriate,
// and patching up branches to |original_region_entry_block| to refer to its
// clone. Parameters |region_output_ids| and |output_id_to_fresh_id_map| are
- // used to determine what the function should return.
+ // used to determine what the function should return. Parameter
+ // |output_id_to_type_id| provides the type of each output id.
//
// The |transformation_context| argument allow facts about blocks being
// outlined, e.g. whether they are dead blocks, to be asserted about blocks
@@ -188,6 +189,7 @@
const opt::BasicBlock& original_region_exit_block,
const std::set<opt::BasicBlock*>& region_blocks,
const std::vector<uint32_t>& region_output_ids,
+ const std::map<uint32_t, uint32_t>& output_id_to_type_id,
const std::map<uint32_t, uint32_t>& output_id_to_fresh_id_map,
opt::IRContext* ir_context, opt::Function* outlined_function) const;
diff --git a/test/fuzz/transformation_outline_function_test.cpp b/test/fuzz/transformation_outline_function_test.cpp
index c567680..6c0bff4 100644
--- a/test/fuzz/transformation_outline_function_test.cpp
+++ b/test/fuzz/transformation_outline_function_test.cpp
@@ -2531,6 +2531,98 @@
ApplyAndCheckFreshIds(transformation, context.get(), &transformation_context);
}
+TEST(TransformationOutlineFunctionTest, SkipVoidOutputId) {
+ std::string shader = R"(
+ OpCapability Shader
+ %1 = OpExtInstImport "GLSL.std.450"
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Fragment %6 "main"
+ OpExecutionMode %6 OriginUpperLeft
+ OpSource ESSL 310
+ %2 = OpTypeVoid
+ %3 = OpTypeFunction %2
+ %21 = OpTypeBool
+ %81 = OpConstantTrue %21
+ %6 = OpFunction %2 None %3
+ %7 = OpLabel
+ OpBranch %80
+ %80 = OpLabel
+ %84 = OpFunctionCall %2 %87
+ OpBranch %90
+ %90 = OpLabel
+ %86 = OpPhi %2 %84 %80
+ OpReturn
+ OpFunctionEnd
+ %87 = OpFunction %2 None %3
+ %88 = OpLabel
+ OpReturn
+ OpFunctionEnd
+ )";
+
+ const auto env = SPV_ENV_UNIVERSAL_1_5;
+ const auto consumer = nullptr;
+ const auto context = BuildModule(env, consumer, shader, kFuzzAssembleOption);
+ spvtools::ValidatorOptions validator_options;
+ ASSERT_TRUE(fuzzerutil::IsValidAndWellFormed(context.get(), validator_options,
+ kConsoleMessageConsumer));
+ TransformationContext transformation_context(
+ MakeUnique<FactManager>(context.get()), validator_options);
+ TransformationOutlineFunction transformation(
+ /*entry_block*/ 80,
+ /*exit_block*/ 80,
+ /*new_function_struct_return_type_id*/ 300,
+ /*new_function_type_id*/ 301,
+ /*new_function_id*/ 302,
+ /*new_function_region_entry_block*/ 304,
+ /*new_caller_result_id*/ 305,
+ /*new_callee_result_id*/ 306,
+ /*input_id_to_fresh_id*/ {},
+ /*output_id_to_fresh_id*/ {{84, 307}});
+
+ ASSERT_TRUE(
+ transformation.IsApplicable(context.get(), transformation_context));
+ ApplyAndCheckFreshIds(transformation, context.get(), &transformation_context);
+ ASSERT_TRUE(fuzzerutil::IsValidAndWellFormed(context.get(), validator_options,
+ kConsoleMessageConsumer));
+
+ std::string after_transformation = R"(
+ OpCapability Shader
+ %1 = OpExtInstImport "GLSL.std.450"
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Fragment %6 "main"
+ OpExecutionMode %6 OriginUpperLeft
+ OpSource ESSL 310
+ %2 = OpTypeVoid
+ %3 = OpTypeFunction %2
+ %21 = OpTypeBool
+ %81 = OpConstantTrue %21
+ %300 = OpTypeStruct
+ %301 = OpTypeFunction %300
+ %6 = OpFunction %2 None %3
+ %7 = OpLabel
+ OpBranch %80
+ %80 = OpLabel
+ %305 = OpFunctionCall %300 %302
+ %84 = OpUndef %2
+ OpBranch %90
+ %90 = OpLabel
+ %86 = OpPhi %2 %84 %80
+ OpReturn
+ OpFunctionEnd
+ %87 = OpFunction %2 None %3
+ %88 = OpLabel
+ OpReturn
+ OpFunctionEnd
+ %302 = OpFunction %300 None %301
+ %304 = OpLabel
+ %307 = OpFunctionCall %2 %87
+ %306 = OpCompositeConstruct %300
+ OpReturnValue %306
+ OpFunctionEnd
+ )";
+ ASSERT_TRUE(IsEqual(env, after_transformation, context.get()));
+}
+
TEST(TransformationOutlineFunctionTest, Miscellaneous1) {
// This tests outlining of some non-trivial code, and also tests the way
// overflow ids are used by the transformation.
