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.

source/fuzz/transformation_duplicate_region_with_selection.cpp

@@ -167,9 +167,11 @@ bool TransformationDuplicateRegionWithSelection::IsApplicable(
           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.
+      // the loop header. However, we would like to exclude this situation,
-      if (continue_target != exit_block &&
+      // since it would be impossible for the modified exit block to branch to
-          region_set.count(&block) != region_set.count(continue_target)) {
+      // 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 @@ void TransformationDuplicateRegionWithSelection::Apply(
   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 @@ void TransformationDuplicateRegionWithSelection::Apply(
   }
 
   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 @@ void TransformationDuplicateRegionWithSelection::Apply(
                                                 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 @@ void TransformationDuplicateRegionWithSelection::Apply(
           {{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

test/fuzz/transformation_duplicate_region_with_selection_test.cpp

@@ -1599,6 +1599,87 @@ TEST(TransformationDuplicateRegionWithSelectionTest,
   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