From a8d7062fe5e27678686b41a4f21385db4f5f67b5 Mon Sep 17 00:00:00 2001
From: Alastair Donaldson <afdx@google.com>
Date: Wed, 21 Oct 2020 21:26:59 +0100
Subject: [PATCH] spirv-fuzz: Don't flatten conditional if condition is
 irrelevant (#3944)

Fixes #3942.
---
 ...zzer_pass_flatten_conditional_branches.cpp |  7 +++-
 ...nsformation_flatten_conditional_branch.cpp |  9 ++++
 ...ransformation_flatten_conditional_branch.h |  2 +
 ...mation_flatten_conditional_branch_test.cpp | 41 +++++++++++++++++++
 4 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/source/fuzz/fuzzer_pass_flatten_conditional_branches.cpp b/source/fuzz/fuzzer_pass_flatten_conditional_branches.cpp
index d8f50c34..c7c29331 100644
--- a/source/fuzz/fuzzer_pass_flatten_conditional_branches.cpp
+++ b/source/fuzz/fuzzer_pass_flatten_conditional_branches.cpp
@@ -47,10 +47,13 @@ void FuzzerPassFlattenConditionalBranches::Apply() {
         continue;
       }
 
-      // Only consider this block if it is the header of a conditional.
+      // Only consider this block if it is the header of a conditional, with a
+      // non-irrelevant condition.
       if (block.GetMergeInst() &&
           block.GetMergeInst()->opcode() == SpvOpSelectionMerge &&
-          block.terminator()->opcode() == SpvOpBranchConditional) {
+          block.terminator()->opcode() == SpvOpBranchConditional &&
+          !GetTransformationContext()->GetFactManager()->IdIsIrrelevant(
+              block.terminator()->GetSingleWordInOperand(0))) {
         selection_headers.emplace_back(&block);
       }
     }
diff --git a/source/fuzz/transformation_flatten_conditional_branch.cpp b/source/fuzz/transformation_flatten_conditional_branch.cpp
index ecb2fc58..83b98d05 100644
--- a/source/fuzz/transformation_flatten_conditional_branch.cpp
+++ b/source/fuzz/transformation_flatten_conditional_branch.cpp
@@ -57,6 +57,15 @@ bool TransformationFlattenConditionalBranch::IsApplicable(
     return false;
   }
 
+  // The branch condition cannot be irrelevant: we will make reference to it
+  // multiple times and we need to be guaranteed that these references will
+  // yield the same result; if they are replaced by other ids that will not
+  // work.
+  if (transformation_context.GetFactManager()->IdIsIrrelevant(
+          header_block->terminator()->GetSingleWordInOperand(0))) {
+    return false;
+  }
+
   std::set<uint32_t> used_fresh_ids;
 
   // If ids have been provided to be used as vector guards for OpSelect
diff --git a/source/fuzz/transformation_flatten_conditional_branch.h b/source/fuzz/transformation_flatten_conditional_branch.h
index b8ba7e3b..2d5e8d76 100644
--- a/source/fuzz/transformation_flatten_conditional_branch.h
+++ b/source/fuzz/transformation_flatten_conditional_branch.h
@@ -35,6 +35,8 @@ class TransformationFlattenConditionalBranch : public Transformation {
 
   // - |message_.header_block_id| must be the label id of a reachable selection
   //   header, which ends with an OpBranchConditional instruction.
+  // - The condition of the OpBranchConditional instruction must not be an
+  //   irrelevant id.
   // - The header block and the merge block must describe a single-entry,
   //   single-exit region.
   // - The region must not contain barrier or OpSampledImage instructions.
diff --git a/test/fuzz/transformation_flatten_conditional_branch_test.cpp b/test/fuzz/transformation_flatten_conditional_branch_test.cpp
index 0930949f..9f9027b3 100644
--- a/test/fuzz/transformation_flatten_conditional_branch_test.cpp
+++ b/test/fuzz/transformation_flatten_conditional_branch_test.cpp
@@ -1846,6 +1846,47 @@ TEST(TransformationFlattenConditionalBranchTest, ApplicablePhiToSelectMatrix) {
   ASSERT_TRUE(IsEqual(env, expected_shader, context.get()));
 }
 
+TEST(TransformationFlattenConditionalBranchTest,
+     InapplicableConditionIsIrrelevant) {
+  std::string shader = R"(
+               OpCapability Shader
+          %1 = OpExtInstImport "GLSL.std.450"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Fragment %4 "main"
+               OpExecutionMode %4 OriginUpperLeft
+               OpSource ESSL 320
+          %2 = OpTypeVoid
+          %3 = OpTypeFunction %2
+          %6 = OpTypeBool
+          %7 = OpConstantTrue %6
+         %10 = OpTypeInt 32 1
+          %4 = OpFunction %2 None %3
+          %5 = OpLabel
+               OpSelectionMerge %9 None
+               OpBranchConditional %7 %8 %9
+          %8 = OpLabel
+               OpBranch %9
+          %9 = OpLabel
+               OpReturn
+               OpFunctionEnd
+  )";
+
+  const auto env = SPV_ENV_UNIVERSAL_1_3;
+  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);
+
+  transformation_context.GetFactManager()->AddFactIdIsIrrelevant(7);
+
+  // Inapplicable because the branch condition, %7, is irrelevant.
+  ASSERT_FALSE(TransformationFlattenConditionalBranch(5, true, 0, 0, 0, {})
+                   .IsApplicable(context.get(), transformation_context));
+}
+
 }  // namespace
 }  // namespace fuzz
 }  // namespace spvtools