spirv-fuzz: Support adding dead break from back-edge block (KhronosGr…

…oup#3519)

Fixes KhronosGroup#2577.

source/fuzz/fuzzer_util.cpp

@@ -156,6 +156,25 @@ void AddUnreachableEdgeAndUpdateOpPhis(
   }
 }
 
+bool BlockIsBackEdge(opt::IRContext* context, uint32_t block_id,
+                     uint32_t loop_header_id) {
+  auto block = context->cfg()->block(block_id);
+  auto loop_header = context->cfg()->block(loop_header_id);
+
+  // |block| and |loop_header| must be defined, |loop_header| must be in fact
+  // loop header and |block| must branch to it.
+  if (!(block && loop_header && loop_header->IsLoopHeader() &&
+        block->IsSuccessor(loop_header))) {
+    return false;
+  }
+
+  // |block_id| must be reachable and be dominated by |loop_header|.
+  opt::DominatorAnalysis* dominator_analysis =
+      context->GetDominatorAnalysis(loop_header->GetParent());
+  return dominator_analysis->IsReachable(block_id) &&
+         dominator_analysis->Dominates(loop_header_id, block_id);
+}
+
 bool BlockIsInLoopContinueConstruct(opt::IRContext* context, uint32_t block_id,
                                     uint32_t maybe_loop_header_id) {
   // We deem a block to be part of a loop's continue construct if the loop's

source/fuzz/fuzzer_util.h

@@ -64,6 +64,12 @@ void AddUnreachableEdgeAndUpdateOpPhis(
     bool condition_value,
     const google::protobuf::RepeatedField<google::protobuf::uint32>& phi_ids);
 
+// Returns true if and only if |loop_header_id| is a loop header and
+// |block_id| is a reachable block branching to and dominated by
+// |loop_header_id|.
+bool BlockIsBackEdge(opt::IRContext* context, uint32_t block_id,
+                     uint32_t loop_header_id);
+
 // Returns true if and only if |maybe_loop_header_id| is a loop header and
 // |block_id| is in the continue construct of the associated loop.
 bool BlockIsInLoopContinueConstruct(opt::IRContext* context, uint32_t block_id,

source/fuzz/transformation_add_dead_break.cpp

@@ -85,12 +85,10 @@ bool TransformationAddDeadBreak::AddingBreakRespectsStructuredControlFlow(
     // loop as part of the loop, but it is not legal to jump from a loop's
     // continue construct to the loop's merge (except from the back-edge block),
     // so we need to check for this case.
-    //
-    // TODO(https://github.com/KhronosGroup/SPIRV-Tools/issues/2577): We do not
-    //  currently allow a dead break from a back edge block, but we could and
-    //  ultimately should.
     return !fuzzerutil::BlockIsInLoopContinueConstruct(
-        ir_context, message_.from_block(), containing_construct);
+               ir_context, message_.from_block(), containing_construct) ||
+           fuzzerutil::BlockIsBackEdge(ir_context, message_.from_block(),
+                                       containing_construct);
   }
 
   // Case (3) holds if and only if |to_block| is the merge block for this
@@ -102,7 +100,9 @@ bool TransformationAddDeadBreak::AddingBreakRespectsStructuredControlFlow(
       message_.to_block() ==
           ir_context->cfg()->block(containing_loop_header)->MergeBlockId()) {
     return !fuzzerutil::BlockIsInLoopContinueConstruct(
-        ir_context, message_.from_block(), containing_loop_header);
+               ir_context, message_.from_block(), containing_loop_header) ||
+           fuzzerutil::BlockIsBackEdge(ir_context, message_.from_block(),
+                                       containing_loop_header);
   }
   return false;
 }

test/fuzz/transformation_add_dead_break_test.cpp

@@ -1196,15 +1196,15 @@ TEST(TransformationAddDeadBreakTest, BreakOutOfLoopNest) {
       TransformationAddDeadBreak(header_for_j, merge_do_while, true, {})
           .IsApplicable(context.get(), transformation_context));
 
-  // Not OK to break loop from its continue construct
+  // Not OK to break loop from its continue construct, except from the back-edge
+  // block.
   ASSERT_FALSE(
       TransformationAddDeadBreak(continue_do_while, merge_do_while, true, {})
           .IsApplicable(context.get(), transformation_context));
-  ASSERT_FALSE(
-      TransformationAddDeadBreak(continue_for_j, merge_for_j, false, {})
-          .IsApplicable(context.get(), transformation_context));
-  ASSERT_FALSE(TransformationAddDeadBreak(continue_for_i, merge_for_i, true, {})
-                   .IsApplicable(context.get(), transformation_context));
+  ASSERT_TRUE(TransformationAddDeadBreak(continue_for_j, merge_for_j, false, {})
+                  .IsApplicable(context.get(), transformation_context));
+  ASSERT_TRUE(TransformationAddDeadBreak(continue_for_i, merge_for_i, true, {})
+                  .IsApplicable(context.get(), transformation_context));
 
   // Not OK to break out of multiple non-loop constructs if not breaking to a
   // loop merge
@@ -1468,11 +1468,117 @@ TEST(TransformationAddDeadBreakTest, NoBreakFromContinueConstruct) {
   TransformationContext transformation_context(&fact_manager,
                                                validator_options);
 
-  // Not OK to break loop from its continue construct
+  // Not OK to break loop from its continue construct, except from the back-edge
+  // block.
   ASSERT_FALSE(TransformationAddDeadBreak(13, 12, true, {})
                    .IsApplicable(context.get(), transformation_context));
-  ASSERT_FALSE(TransformationAddDeadBreak(23, 12, true, {})
-                   .IsApplicable(context.get(), transformation_context));
+  ASSERT_TRUE(TransformationAddDeadBreak(23, 12, true, {})
+                  .IsApplicable(context.get(), transformation_context));
+}
+
+TEST(TransformationAddDeadBreakTest, BreakFromBackEdgeBlock) {
+  std::string reference_shader = R"(
+               OpCapability Shader
+          %1 = OpExtInstImport "GLSL.std.450"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Vertex %10 "main"
+
+; Types
+          %2 = OpTypeVoid
+          %3 = OpTypeFunction %2
+          %4 = OpTypeInt 32 0
+          %5 = OpTypeBool
+          %6 = OpTypePointer Function %4
+
+; Constants
+          %7 = OpConstant %4 0
+          %8 = OpConstant %4 1
+          %9 = OpConstantTrue %5
+
+; main function
+         %10 = OpFunction %2 None %3
+         %11 = OpLabel
+         %12 = OpVariable %6 Function
+               OpStore %12 %7
+               OpBranch %13
+         %13 = OpLabel
+               OpLoopMerge %21 %18 None ; structured loop
+               OpBranch %14
+         %14 = OpLabel
+         %15 = OpLoad %4 %12
+         %16 = OpULessThan %5 %15 %8 ; i < 1 ?
+               OpBranchConditional %16 %17 %21 ; body or break
+         %17 = OpLabel ; body
+               OpBranch %18
+         %18 = OpLabel ; continue target does not strictly dominates the back-edge block
+         %19 = OpLoad %4 %12
+         %20 = OpIAdd %4 %19 %8 ; ++i
+               OpStore %12 %20
+               OpBranch %13
+         %21 = OpLabel
+               OpReturn
+               OpFunctionEnd
+  )";
+
+  const auto env = SPV_ENV_UNIVERSAL_1_5;
+  const auto consumer = nullptr;
+  const auto context =
+      BuildModule(env, consumer, reference_shader, kFuzzAssembleOption);
+  ASSERT_TRUE(IsValid(env, context.get()));
+
+  FactManager fact_manager;
+  spvtools::ValidatorOptions validator_options;
+  TransformationContext transformation_context(&fact_manager,
+                                               validator_options);
+
+  auto transformation = TransformationAddDeadBreak(18, 21, true, {});
+  transformation.Apply(context.get(), &transformation_context);
+
+  std::string variant_shader = R"(
+               OpCapability Shader
+          %1 = OpExtInstImport "GLSL.std.450"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Vertex %10 "main"
+
+; Types
+          %2 = OpTypeVoid
+          %3 = OpTypeFunction %2
+          %4 = OpTypeInt 32 0
+          %5 = OpTypeBool
+          %6 = OpTypePointer Function %4
+
+; Constants
+          %7 = OpConstant %4 0
+          %8 = OpConstant %4 1
+          %9 = OpConstantTrue %5
+
+; main function
+         %10 = OpFunction %2 None %3
+         %11 = OpLabel
+         %12 = OpVariable %6 Function
+               OpStore %12 %7
+               OpBranch %13
+         %13 = OpLabel
+               OpLoopMerge %21 %18 None ; structured loop
+               OpBranch %14
+         %14 = OpLabel
+         %15 = OpLoad %4 %12
+         %16 = OpULessThan %5 %15 %8 ; i < 1 ?
+               OpBranchConditional %16 %17 %21 ; body or break
+         %17 = OpLabel ; body
+               OpBranch %18
+         %18 = OpLabel ; continue target does not strictly dominates the back-edge block
+         %19 = OpLoad %4 %12
+         %20 = OpIAdd %4 %19 %8 ; ++i
+               OpStore %12 %20
+               OpBranchConditional %9 %13 %21
+         %21 = OpLabel
+               OpReturn
+               OpFunctionEnd
+  )";
+
+  ASSERT_TRUE(IsValid(env, context.get()));
+  ASSERT_TRUE(IsEqual(env, variant_shader, context.get()));
 }
 
 TEST(TransformationAddDeadBreakTest, SelectionInContinueConstruct) {