spirv-fuzz: Fix TransformationDuplicateRegionWithSelection (#3815)
Introduces two changes:
- duplicated_exit_region refers to a correct block, regardless of the order
of the blocks in the enclosing function.
- Exclude the case where the continue target is the exit block.
diff --git a/source/fuzz/transformation_duplicate_region_with_selection.cpp b/source/fuzz/transformation_duplicate_region_with_selection.cpp
index 3b6c1df..8c72130 100644
--- a/source/fuzz/transformation_duplicate_region_with_selection.cpp
+++ b/source/fuzz/transformation_duplicate_region_with_selection.cpp
@@ -167,9 +167,11 @@
ir_context->cfg()->block(loop_merge->GetSingleWordOperand(1));
// The continue target is a single-entry, single-exit region. Therefore,
// if the continue target is the exit block, the region might not contain
- // the loop header.
- if (continue_target != exit_block &&
- region_set.count(&block) != region_set.count(continue_target)) {
+ // the loop header. However, we would like to exclude this situation,
+ // since it would be impossible for the modified exit block to branch to
+ // the new selection merge block. In this scenario the exit block is
+ // required to branch to the loop header.
+ if (region_set.count(&block) != region_set.count(continue_target)) {
return false;
}
}
@@ -325,7 +327,7 @@
uint32_t entry_block_pred_id =
ir_context->get_instr_block(entry_block_preds[0])->id();
// Update all the OpPhi instructions in the |entry_block|. Change every
- // occurence of |entry_block_pred_id| to the id of |new_entry|, because we
+ // occurrence of |entry_block_pred_id| to the id of |new_entry|, because we
// will insert |new_entry| before |entry_block|.
for (auto& instr : *entry_block) {
if (instr.opcode() == SpvOpPhi) {
@@ -345,6 +347,7 @@
}
opt::BasicBlock* previous_block = nullptr;
+ opt::BasicBlock* duplicated_exit_block = nullptr;
// Iterate over all blocks of the function to duplicate blocks of the original
// region and their instructions.
for (auto& block : blocks) {
@@ -414,12 +417,13 @@
exit_block);
}
previous_block = duplicated_block_ptr;
+ if (block == exit_block) {
+ // After execution of the loop, this variable stores a pointer to the last
+ // duplicated block.
+ duplicated_exit_block = duplicated_block_ptr;
+ }
}
- // After execution of the loop, this variable stores a pointer to the last
- // duplicated block.
- auto duplicated_exit_block = previous_block;
-
for (auto& block : region_blocks) {
for (auto& instr : *block) {
if (instr.result_id() != 0 &&
@@ -500,7 +504,7 @@
{{SPV_OPERAND_TYPE_ID, {message_.merge_label_fresh_id()}}}));
exit_block->AddInstruction(MakeUnique<opt::Instruction>(merge_branch_instr));
duplicated_exit_block->AddInstruction(
- MakeUnique<opt::Instruction>(merge_branch_instr));
+ std::unique_ptr<opt::Instruction>(merge_branch_instr.Clone(ir_context)));
// Execution needs to start in the |new_entry_block|. Change all
// the uses of |entry_block_label_instr| outside of the original
diff --git a/test/fuzz/transformation_duplicate_region_with_selection_test.cpp b/test/fuzz/transformation_duplicate_region_with_selection_test.cpp
index 6539c4a..c1fac9a 100644
--- a/test/fuzz/transformation_duplicate_region_with_selection_test.cpp
+++ b/test/fuzz/transformation_duplicate_region_with_selection_test.cpp
@@ -1599,6 +1599,87 @@
ASSERT_TRUE(IsEqual(env, expected_shader, context.get()));
}
+TEST(TransformationDuplicateRegionWithSelectionTest,
+ ContinueExitBlockNotApplicable) {
+ // This test handles a case where the exit block is the continue target and
+ // the transformation is not applicable.
+
+ std::string shader = R"(
+ OpCapability Shader
+ %1 = OpExtInstImport "GLSL.std.450"
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Fragment %4 "main"
+ OpExecutionMode %4 OriginUpperLeft
+ OpSource ESSL 310
+ OpName %4 "main"
+ OpName %8 "s"
+ OpName %10 "i"
+ %2 = OpTypeVoid
+ %3 = OpTypeFunction %2
+ %6 = OpTypeInt 32 1
+ %7 = OpTypePointer Function %6
+ %9 = OpConstant %6 0
+ %17 = OpConstant %6 10
+ %18 = OpTypeBool
+ %24 = OpConstant %6 5
+ %30 = OpConstant %6 1
+ %50 = OpConstantTrue %18
+ %4 = OpFunction %2 None %3
+ %5 = OpLabel
+ %8 = OpVariable %7 Function
+ %10 = OpVariable %7 Function
+ OpStore %8 %9
+ OpStore %10 %9
+ OpBranch %11
+ %11 = OpLabel
+ OpLoopMerge %13 %14 None
+ OpBranch %15
+ %15 = OpLabel
+ %16 = OpLoad %6 %10
+ %19 = OpSLessThan %18 %16 %17
+ OpBranchConditional %19 %12 %13
+ %12 = OpLabel
+ %20 = OpLoad %6 %10
+ %21 = OpLoad %6 %8
+ %22 = OpIAdd %6 %21 %20
+ OpStore %8 %22
+ %23 = OpLoad %6 %10
+ %25 = OpIEqual %18 %23 %24
+ OpSelectionMerge %27 None
+ OpBranchConditional %25 %26 %27
+ %26 = OpLabel
+ OpBranch %13
+ %27 = OpLabel
+ OpBranch %14
+ %14 = OpLabel
+ %29 = OpLoad %6 %10
+ %31 = OpIAdd %6 %29 %30
+ OpStore %10 %31
+ OpBranch %11
+ %13 = OpLabel
+ OpReturn
+ OpFunctionEnd
+ )";
+
+ const auto env = SPV_ENV_UNIVERSAL_1_4;
+ const auto consumer = nullptr;
+ const auto context = BuildModule(env, consumer, shader, kFuzzAssembleOption);
+ ASSERT_TRUE(IsValid(env, context.get()));
+
+ FactManager fact_manager;
+ spvtools::ValidatorOptions validator_options;
+ TransformationContext transformation_context(&fact_manager,
+ validator_options);
+
+ TransformationDuplicateRegionWithSelection transformation_bad =
+ TransformationDuplicateRegionWithSelection(
+ 500, 50, 501, 27, 14, {{27, 101}, {14, 102}}, {{29, 201}, {31, 202}},
+ {{29, 301}, {31, 302}});
+
+ ASSERT_FALSE(
+ transformation_bad.IsApplicable(context.get(), transformation_context));
+}
+
} // namespace
} // namespace fuzz
} // namespace spvtools
