Squashed 'third_party/SPIRV-Tools/' changes from f7da52775..a61d07a72

source/fuzz/fact_manager/data_synonym_and_id_equation_facts.cpp

@@ -176,12 +176,6 @@ void DataSynonymAndIdEquationFacts::AddEquationFactRecursive(
             // We can thus infer "a = d"
             AddDataSynonymFactRecursive(lhs_dd, *equation.operands[0]);
           }
-          if (synonymous_.IsEquivalent(*equation.operands[0], *rhs_dds[1])) {
-            // Equation form: "a = (c - e) + c"
-            // We can thus infer "a = -e"
-            AddEquationFactRecursive(lhs_dd, SpvOpSNegate,
-                                     {equation.operands[1]});
-          }
         }
       }
       for (const auto& equation : GetEquations(rhs_dds[1])) {

source/fuzz/fuzzer.cpp

@@ -107,8 +107,6 @@ namespace fuzz {
 namespace {
 const uint32_t kIdBoundGap = 100;
 
-const uint32_t kTransformationLimit = 2000;
-
 }  // namespace
 
 Fuzzer::Fuzzer(spv_target_env target_env, MessageConsumer consumer,
@@ -367,21 +365,37 @@ bool Fuzzer::ShouldContinueFuzzing() {
   // that fuzzing stops if the number of repeated passes hits the limit on the
   // number of transformations that can be applied.
   assert(
-      num_repeated_passes_applied_ <= kTransformationLimit &&
+      num_repeated_passes_applied_ <=
+          fuzzer_context_->GetTransformationLimit() &&
       "The number of repeated passes applied must not exceed its upper limit.");
-  if (num_repeated_passes_applied_ == kTransformationLimit) {
+  if (ir_context_->module()->id_bound() >= fuzzer_context_->GetIdBoundLimit()) {
+    return false;
+  }
+  if (num_repeated_passes_applied_ ==
+      fuzzer_context_->GetTransformationLimit()) {
     // Stop because fuzzing has got stuck.
     return false;
   }
   auto transformations_applied_so_far =
       static_cast<uint32_t>(transformation_sequence_out_.transformation_size());
-  if (transformations_applied_so_far >= kTransformationLimit) {
+  if (transformations_applied_so_far >=
+      fuzzer_context_->GetTransformationLimit()) {
     // Stop because we have reached the transformation limit.
     return false;
   }
+  // If we have applied T transformations so far, and the limit on the number of
+  // transformations to apply is L (where T < L), the chance that we will
+  // continue fuzzing is:
+  //
+  //     1 - T/(2*L)
+  //
+  // That is, the chance of continuing decreases as more transformations are
+  // applied.  Using 2*L instead of L increases the number of transformations
+  // that are applied on average.
   auto chance_of_continuing = static_cast<uint32_t>(
       100.0 * (1.0 - (static_cast<double>(transformations_applied_so_far) /
-                      static_cast<double>(kTransformationLimit))));
+                      (2.0 * static_cast<double>(
+                                 fuzzer_context_->GetTransformationLimit())))));
   if (!fuzzer_context_->ChoosePercentage(chance_of_continuing)) {
     // We have probabilistically decided to stop.
     return false;

source/fuzz/fuzzer_context.cpp

@@ -20,14 +20,19 @@ namespace spvtools {
 namespace fuzz {
 
 namespace {
+
+// Limits to help control the overall fuzzing process and rein in individual
+// fuzzer passes.
+const uint32_t kIdBoundLimit = 50000;
+const uint32_t kTransformationLimit = 2000;
+
 // Default <minimum, maximum> pairs of probabilities for applying various
 // transformations. All values are percentages. Keep them in alphabetical order.
-
 const std::pair<uint32_t, uint32_t>
-    kChanceOfAcceptingRepeatedPassRecommendation = {70, 100};
+    kChanceOfAcceptingRepeatedPassRecommendation = {50, 80};
 const std::pair<uint32_t, uint32_t> kChanceOfAddingAccessChain = {5, 50};
-const std::pair<uint32_t, uint32_t> kChanceOfAddingAnotherPassToPassLoop = {85,
-                                                                            95};
+const std::pair<uint32_t, uint32_t> kChanceOfAddingAnotherPassToPassLoop = {50,
+                                                                            90};
 const std::pair<uint32_t, uint32_t> kChanceOfAddingAnotherStructField = {20,
                                                                          90};
 const std::pair<uint32_t, uint32_t> kChanceOfAddingArrayOrStructType = {20, 90};
@@ -392,5 +397,11 @@ FuzzerContext::GetRandomSynonymType() {
   return static_cast<protobufs::TransformationAddSynonym::SynonymType>(result);
 }
 
+uint32_t FuzzerContext::GetIdBoundLimit() const { return kIdBoundLimit; }
+
+uint32_t FuzzerContext::GetTransformationLimit() const {
+  return kTransformationLimit;
+}
+
 }  // namespace fuzz
 }  // namespace spvtools

source/fuzz/fuzzer_pass_add_bit_instruction_synonyms.cpp

@@ -35,6 +35,13 @@ void FuzzerPassAddBitInstructionSynonyms::Apply() {
   for (auto& function : *GetIRContext()->module()) {
     for (auto& block : function) {
       for (auto& instruction : block) {
+        // This fuzzer pass can add a *lot* of ids.  We bail out early if we hit
+        // the recommended id limit.
+        if (GetIRContext()->module()->id_bound() >=
+            GetFuzzerContext()->GetIdBoundLimit()) {
+          return;
+        }
+
         // Randomly decides whether the transformation will be applied.
         if (!GetFuzzerContext()->ChoosePercentage(
                 GetFuzzerContext()->GetChanceOfAddingBitInstructionSynonym())) {

source/fuzz/fuzzer_pass_add_composite_extract.cpp

@@ -91,35 +91,29 @@ void FuzzerPassAddCompositeExtract::Apply() {
                   available_composites)];
           composite_id = inst->result_id();
 
-          const auto* type =
-              GetIRContext()->get_type_mgr()->GetType(inst->type_id());
-          assert(type && "Composite instruction has invalid type id");
-
+          auto type_id = inst->type_id();
           do {
             uint32_t number_of_members = 0;
 
-            if (const auto* array_type = type->AsArray()) {
-              const auto* type_inst =
-                  GetIRContext()->get_def_use_mgr()->GetDef(inst->type_id());
-              assert(type_inst && "Type instruction must exist");
-
-              number_of_members =
-                  fuzzerutil::GetArraySize(*type_inst, GetIRContext());
-              type = array_type->element_type();
-            } else if (const auto* vector_type = type->AsVector()) {
-              number_of_members = vector_type->element_count();
-              type = vector_type->element_type();
-            } else if (const auto* matrix_type = type->AsMatrix()) {
-              number_of_members = matrix_type->element_count();
-              type = matrix_type->element_type();
-            } else if (const auto* struct_type = type->AsStruct()) {
-              number_of_members =
-                  static_cast<uint32_t>(struct_type->element_types().size());
-              // The next value of |type| will be assigned when we know the
-              // index of the OpTypeStruct's operand.
-            } else {
-              assert(false && "|inst| is not a composite");
-              return;
+            const auto* type_inst =
+                GetIRContext()->get_def_use_mgr()->GetDef(type_id);
+            assert(type_inst && "Composite instruction has invalid type id");
+
+            switch (type_inst->opcode()) {
+              case SpvOpTypeArray:
+                number_of_members =
+                    fuzzerutil::GetArraySize(*type_inst, GetIRContext());
+                break;
+              case SpvOpTypeVector:
+              case SpvOpTypeMatrix:
+                number_of_members = type_inst->GetSingleWordInOperand(1);
+                break;
+              case SpvOpTypeStruct:
+                number_of_members = type_inst->NumInOperands();
+                break;
+              default:
+                assert(false && "|type_inst| is not a composite");
+                return;
             }
 
             if (number_of_members == 0) {
@@ -130,10 +124,21 @@ void FuzzerPassAddCompositeExtract::Apply() {
                 GetFuzzerContext()->GetRandomCompositeExtractIndex(
                     number_of_members));
 
-            if (const auto* struct_type = type->AsStruct()) {
-              type = struct_type->element_types()[indices.back()];
+            switch (type_inst->opcode()) {
+              case SpvOpTypeArray:
+              case SpvOpTypeVector:
+              case SpvOpTypeMatrix:
+                type_id = type_inst->GetSingleWordInOperand(0);
+                break;
+              case SpvOpTypeStruct:
+                type_id = type_inst->GetSingleWordInOperand(indices.back());
+                break;
+              default:
+                assert(false && "|type_inst| is not a composite");
+                return;
             }
-          } while (fuzzerutil::IsCompositeType(type) &&
+          } while (fuzzerutil::IsCompositeType(
+                       GetIRContext()->get_type_mgr()->GetType(type_id)) &&
                    GetFuzzerContext()->ChoosePercentage(
                        GetFuzzerContext()
                            ->GetChanceOfGoingDeeperToExtractComposite()));

source/fuzz/fuzzer_pass_flatten_conditional_branches.cpp

@@ -71,7 +71,8 @@ void FuzzerPassFlattenConditionalBranches::Apply() {
       // Do not consider this header if the conditional cannot be flattened.
       if (!TransformationFlattenConditionalBranch::
               GetProblematicInstructionsIfConditionalCanBeFlattened(
-                  GetIRContext(), header, &instructions_that_need_ids)) {
+                  GetIRContext(), header, *GetTransformationContext(),
+                  &instructions_that_need_ids)) {
         continue;
       }
 
@@ -214,7 +215,7 @@ void FuzzerPassFlattenConditionalBranches::Apply() {
           }
         }
 
-        wrappers_info.emplace_back(wrapper_info);
+        wrappers_info.push_back(std::move(wrapper_info));
       }
 
       // Apply the transformation, evenly choosing whether to lay out the true

source/fuzz/pass_management/repeated_pass_manager_random_with_recommendations.cpp

@@ -53,6 +53,8 @@ FuzzerPass* RepeatedPassManagerRandomWithRecommendations::ChoosePass(
     result = recommended_passes_.front();
     recommended_passes_.pop_front();
   }
+  assert(result != nullptr && "A pass must have been chosen.");
+  last_pass_choice_ = result;
   return result;
 }
 

source/fuzz/transformation_add_bit_instruction_synonym.cpp

@@ -46,7 +46,8 @@ bool TransformationAddBitInstructionSynonym::IsApplicable(
   // |instruction| must be defined and must be a supported bit instruction.
   if (!instruction || (instruction->opcode() != SpvOpBitwiseOr &&
                        instruction->opcode() != SpvOpBitwiseXor &&
-                       instruction->opcode() != SpvOpBitwiseAnd)) {
+                       instruction->opcode() != SpvOpBitwiseAnd &&
+                       instruction->opcode() != SpvOpNot)) {
     return false;
   }
 

source/fuzz/transformation_flatten_conditional_branch.h

@@ -72,6 +72,7 @@ class TransformationFlattenConditionalBranch : public Transformation {
   // instructions are OpSelectionMerge and OpBranchConditional.
   static bool GetProblematicInstructionsIfConditionalCanBeFlattened(
       opt::IRContext* ir_context, opt::BasicBlock* header,
+      const TransformationContext& transformation_context,
       std::set<opt::Instruction*>* instructions_that_need_ids);
 
   // Returns true iff the given instruction needs a placeholder to be enclosed
@@ -117,14 +118,14 @@ class TransformationFlattenConditionalBranch : public Transformation {
   // |dead_blocks| and |irrelevant_ids| are used to record the ids of blocks
   // and instructions for which dead block and irrelevant id facts should
   // ultimately be created.
-  opt::BasicBlock* EncloseInstructionInConditional(
+  static opt::BasicBlock* EncloseInstructionInConditional(
       opt::IRContext* ir_context,
       const TransformationContext& transformation_context,
       opt::BasicBlock* block, opt::Instruction* instruction,
       const protobufs::SideEffectWrapperInfo& wrapper_info,
       uint32_t condition_id, bool exec_if_cond_true,
       std::vector<uint32_t>* dead_blocks,
-      std::vector<uint32_t>* irrelevant_ids) const;
+      std::vector<uint32_t>* irrelevant_ids);
 
   // Turns every OpPhi instruction of |convergence_block| -- the convergence
   // block for |header_block| (both in |ir_context|) into an OpSelect
@@ -137,10 +138,10 @@ class TransformationFlattenConditionalBranch : public Transformation {
   // |ir_context|, with result id given by |fresh_id|.  The instruction will
   // make a |dimension|-dimensional boolean vector with
   // |branch_condition_operand| at every component.
-  void AddBooleanVectorConstructorToBlock(
+  static void AddBooleanVectorConstructorToBlock(
       uint32_t fresh_id, uint32_t dimension,
       const opt::Operand& branch_condition_operand, opt::IRContext* ir_context,
-      opt::BasicBlock* block) const;
+      opt::BasicBlock* block);
 
   // Returns true if the given instruction either has no side effects or it can
   // be handled by being enclosed in a conditional.

source/opt/debug_info_manager.cpp

@@ -384,7 +384,8 @@ bool DebugInfoManager::IsVariableDebugDeclared(uint32_t variable_id) {
   return dbg_decl_itr != var_id_to_dbg_decl_.end();
 }
 
-void DebugInfoManager::KillDebugDeclares(uint32_t variable_id) {
+bool DebugInfoManager::KillDebugDeclares(uint32_t variable_id) {
+  bool modified = false;
   auto dbg_decl_itr = var_id_to_dbg_decl_.find(variable_id);
   if (dbg_decl_itr != var_id_to_dbg_decl_.end()) {
     // We intentionally copy the list of DebugDeclare instructions because
@@ -394,9 +395,11 @@ void DebugInfoManager::KillDebugDeclares(uint32_t variable_id) {
 
     for (auto* dbg_decl : copy_dbg_decls) {
       context()->KillInst(dbg_decl);
+      modified = true;
     }
     var_id_to_dbg_decl_.erase(dbg_decl_itr);
   }
+  return modified;
 }
 
 uint32_t DebugInfoManager::GetParentScope(uint32_t child_scope) {
@@ -514,16 +517,18 @@ Instruction* DebugInfoManager::AddDebugValueWithIndex(
   return added_dbg_value;
 }
 
-void DebugInfoManager::AddDebugValueIfVarDeclIsVisible(
+bool DebugInfoManager::AddDebugValueIfVarDeclIsVisible(
     Instruction* scope_and_line, uint32_t variable_id, uint32_t value_id,
     Instruction* insert_pos,
     std::unordered_set<Instruction*>* invisible_decls) {
+  assert(scope_and_line != nullptr);
+
   auto dbg_decl_itr = var_id_to_dbg_decl_.find(variable_id);
-  if (dbg_decl_itr == var_id_to_dbg_decl_.end()) return;
+  if (dbg_decl_itr == var_id_to_dbg_decl_.end()) return false;
 
+  bool modified = false;
   for (auto* dbg_decl_or_val : dbg_decl_itr->second) {
-    if (scope_and_line &&
-        !IsDeclareVisibleToInstr(dbg_decl_or_val, scope_and_line)) {
+    if (!IsDeclareVisibleToInstr(dbg_decl_or_val, scope_and_line)) {
       if (invisible_decls) invisible_decls->insert(dbg_decl_or_val);
       continue;
     }
@@ -547,10 +552,11 @@ void DebugInfoManager::AddDebugValueIfVarDeclIsVisible(
                                    kDebugValueOperandLocalVariableIndex),
                                value_id, 0, index_id, insert_before);
     assert(added_dbg_value != nullptr);
-    added_dbg_value->UpdateDebugInfoFrom(scope_and_line ? scope_and_line
-                                                        : dbg_decl_or_val);
+    added_dbg_value->UpdateDebugInfoFrom(scope_and_line);
     AnalyzeDebugInst(added_dbg_value);
+    modified = true;
   }
+  return modified;
 }
 
 bool DebugInfoManager::AddDebugValueForDecl(Instruction* dbg_decl,

source/opt/debug_info_manager.h

@@ -138,14 +138,15 @@ class DebugInfoManager {
   bool IsVariableDebugDeclared(uint32_t variable_id);
 
   // Kills all debug declaration instructions with Deref whose 'Local Variable'
-  // operand is |variable_id|.
-  void KillDebugDeclares(uint32_t variable_id);
+  // operand is |variable_id|. Returns whether it kills an instruction or not.
+  bool KillDebugDeclares(uint32_t variable_id);
 
   // Generates a DebugValue instruction with value |value_id| for every local
   // variable that is in the scope of |scope_and_line| and whose memory is
   // |variable_id| and inserts it after the instruction |insert_pos|.
-  // |invisible_decls| returns DebugDeclares invisible to |scope_and_line|.
-  void AddDebugValueIfVarDeclIsVisible(
+  // Returns whether a DebugValue is added or not. |invisible_decls| returns
+  // DebugDeclares invisible to |scope_and_line|.
+  bool AddDebugValueIfVarDeclIsVisible(
       Instruction* scope_and_line, uint32_t variable_id, uint32_t value_id,
       Instruction* insert_pos,
       std::unordered_set<Instruction*>* invisible_decls);

source/opt/local_single_store_elim_pass.cpp

@@ -148,16 +148,41 @@ bool LocalSingleStoreElimPass::ProcessVariable(Instruction* var_inst) {
         context()->get_type_mgr()->GetType(var_inst->type_id());
     const analysis::Type* store_type = var_type->AsPointer()->pointee_type();
     if (!(store_type->AsStruct() || store_type->AsArray())) {
-      context()->get_debug_info_mgr()->AddDebugValueIfVarDeclIsVisible(
-          nullptr, var_id, store_inst->GetSingleWordInOperand(1), store_inst,
-          nullptr);
-      context()->get_debug_info_mgr()->KillDebugDeclares(var_id);
+      modified |= RewriteDebugDeclares(store_inst, var_id);
     }
   }
 
   return modified;
 }
 
+bool LocalSingleStoreElimPass::RewriteDebugDeclares(Instruction* store_inst,
+                                                    uint32_t var_id) {
+  std::unordered_set<Instruction*> invisible_decls;
+  uint32_t value_id = store_inst->GetSingleWordInOperand(1);
+  bool modified =
+      context()->get_debug_info_mgr()->AddDebugValueIfVarDeclIsVisible(
+          store_inst, var_id, value_id, store_inst, &invisible_decls);
+
+  // For cases like the argument passing for an inlined function, the value
+  // assignment is out of DebugDeclare's scope, but we have to preserve the
+  // value assignment information using DebugValue. Generally, we need
+  // ssa-rewrite analysis to decide a proper value assignment but at this point
+  // we confirm that |var_id| has a single store. We can safely add DebugValue.
+  if (!invisible_decls.empty()) {
+    BasicBlock* store_block = context()->get_instr_block(store_inst);
+    DominatorAnalysis* dominator_analysis =
+        context()->GetDominatorAnalysis(store_block->GetParent());
+    for (auto* decl : invisible_decls) {
+      if (dominator_analysis->Dominates(store_inst, decl)) {
+        context()->get_debug_info_mgr()->AddDebugValueForDecl(decl, value_id);
+        modified = true;
+      }
+    }
+  }
+  modified |= context()->get_debug_info_mgr()->KillDebugDeclares(var_id);
+  return modified;
+}
+
 Instruction* LocalSingleStoreElimPass::FindSingleStoreAndCheckUses(
     Instruction* var_inst, const std::vector<Instruction*>& users) const {
   // Make sure there is exactly 1 store.

source/opt/local_single_store_elim_pass.h

@@ -94,6 +94,10 @@ class LocalSingleStoreElimPass : public Pass {
   bool RewriteLoads(Instruction* store_inst,
                     const std::vector<Instruction*>& uses, bool* all_rewritten);
 
+  // Replaces DebugDeclares of |var_id| with DebugValues using the value
+  // assignment of |store_inst|.
+  bool RewriteDebugDeclares(Instruction* store_inst, uint32_t var_id);
+
   // Extensions supported by this pass.
   std::unordered_set<std::string> extensions_allowlist_;
 };

source/opt/optimizer.cpp

@@ -161,6 +161,7 @@ Optimizer& Optimizer::RegisterPerformancePasses() {
       .RegisterPass(CreateDeadBranchElimPass())
       .RegisterPass(CreateMergeReturnPass())
       .RegisterPass(CreateInlineExhaustivePass())
+      .RegisterPass(CreateEliminateDeadFunctionsPass())
       .RegisterPass(CreateAggressiveDCEPass())
       .RegisterPass(CreatePrivateToLocalPass())
       .RegisterPass(CreateLocalSingleBlockLoadStoreElimPass())

source/opt/ssa_rewrite_pass.cpp

@@ -47,6 +47,7 @@
 #include "source/opcode.h"
 #include "source/opt/cfg.h"
 #include "source/opt/mem_pass.h"
+#include "source/opt/types.h"
 #include "source/util/make_unique.h"
 
 // Debug logging (0: Off, 1-N: Verbosity level).  Replace this with the
@@ -326,32 +327,94 @@ void SSARewriter::ProcessStore(Instruction* inst, BasicBlock* bb) {
 }
 
 bool SSARewriter::ProcessLoad(Instruction* inst, BasicBlock* bb) {
+  // Get the pointer that we are using to load from.
   uint32_t var_id = 0;
   (void)pass_->GetPtr(inst, &var_id);
-  if (pass_->IsTargetVar(var_id)) {
-    // Get the immediate reaching definition for |var_id|.
-    uint32_t val_id = GetReachingDef(var_id, bb);
+
+  // Get the immediate reaching definition for |var_id|.
+  //
+  // In the presence of variable pointers, the reaching definition may be
+  // another pointer.  For example, the following fragment:
+  //
+  //  %2 = OpVariable %_ptr_Input_float Input
+  // %11 = OpVariable %_ptr_Function__ptr_Input_float Function
+  //       OpStore %11 %2
+  // %12 = OpLoad %_ptr_Input_float %11
+  // %13 = OpLoad %float %12
+  //
+  // corresponds to the pseudo-code:
+  //
+  // layout(location = 0) in flat float *%2
+  // float %13;
+  // float *%12;
+  // float **%11;
+  // *%11 = %2;
+  // %12 = *%11;
+  // %13 = *%12;
+  //
+  // which ultimately, should correspond to:
+  //
+  // %13 = *%2;
+  //
+  // During rewriting, the pointer %12 is found to be replaceable by %2 (i.e.,
+  // load_replacement_[12] is 2). However, when processing the load
+  // %13 = *%12, the type of %12's reaching definition is another float
+  // pointer (%2), instead of a float value.
+  //
+  // When this happens, we need to continue looking up the reaching definition
+  // chain until we get to a float value or a non-target var (i.e. a variable
+  // that cannot be SSA replaced, like %2 in this case since it is a function
+  // argument).
+  analysis::DefUseManager* def_use_mgr = pass_->context()->get_def_use_mgr();
+  analysis::TypeManager* type_mgr = pass_->context()->get_type_mgr();
+  analysis::Type* load_type = type_mgr->GetType(inst->type_id());
+  uint32_t val_id = 0;
+  bool found_reaching_def = false;
+  while (!found_reaching_def) {
+    if (!pass_->IsTargetVar(var_id)) {
+      // If the variable we are loading from is not an SSA target (globals,
+      // function parameters), do nothing.
+      return true;
+    }
+
+    val_id = GetReachingDef(var_id, bb);
     if (val_id == 0) {
       return false;
     }
 
-    // Schedule a replacement for the result of this load instruction with
-    // |val_id|. After all the rewriting decisions are made, every use of
-    // this load will be replaced with |val_id|.
-    const uint32_t load_id = inst->result_id();
-    assert(load_replacement_.count(load_id) == 0);
-    load_replacement_[load_id] = val_id;
-    PhiCandidate* defining_phi = GetPhiCandidate(val_id);
-    if (defining_phi) {
-      defining_phi->AddUser(load_id);
+    // If the reaching definition is a pointer type different than the type of
+    // the instruction we are analyzing, then it must be a reference to another
+    // pointer (otherwise, this would be invalid SPIRV).  We continue
+    // de-referencing it by making |val_id| be |var_id|.
+    //
+    // NOTE: if there is no reaching definition instruction, it means |val_id|
+    // is an undef.
+    Instruction* reaching_def_inst = def_use_mgr->GetDef(val_id);
+    if (reaching_def_inst &&
+        !type_mgr->GetType(reaching_def_inst->type_id())->IsSame(load_type)) {
+      var_id = val_id;
+    } else {
+      found_reaching_def = true;
     }
+  }
+
+  // Schedule a replacement for the result of this load instruction with
+  // |val_id|. After all the rewriting decisions are made, every use of
+  // this load will be replaced with |val_id|.
+  uint32_t load_id = inst->result_id();
+  assert(load_replacement_.count(load_id) == 0);
+  load_replacement_[load_id] = val_id;
+  PhiCandidate* defining_phi = GetPhiCandidate(val_id);
+  if (defining_phi) {
+    defining_phi->AddUser(load_id);
+  }
 
 #if SSA_REWRITE_DEBUGGING_LEVEL > 1
-    std::cerr << "\tFound load: "
-              << inst->PrettyPrint(SPV_BINARY_TO_TEXT_OPTION_FRIENDLY_NAMES)
-              << " (replacement for %" << load_id << " is %" << val_id << ")\n";
+  std::cerr << "\tFound load: "
+            << inst->PrettyPrint(SPV_BINARY_TO_TEXT_OPTION_FRIENDLY_NAMES)
+            << " (replacement for %" << load_id << " is %" << val_id << ")\n";
 #endif
-  }
+
   return true;
 }
 
@@ -390,8 +453,8 @@ bool SSARewriter::GenerateSSAReplacements(BasicBlock* bb) {
     }
   }
 
-  // Seal |bb|. This means that all the stores in it have been scanned and it's
-  // ready to feed them into its successors.
+  // Seal |bb|. This means that all the stores in it have been scanned and
+  // it's ready to feed them into its successors.
   SealBlock(bb);
 
 #if SSA_REWRITE_DEBUGGING_LEVEL > 1
@@ -504,8 +567,8 @@ bool SSARewriter::ApplyReplacements() {
   }
 
   // Scan uses for all inserted Phi instructions. Do this separately from the
-  // registration of the Phi instruction itself to avoid trying to analyze uses
-  // of Phi instructions that have not been registered yet.
+  // registration of the Phi instruction itself to avoid trying to analyze
+  // uses of Phi instructions that have not been registered yet.
   for (Instruction* phi_inst : generated_phis) {
     pass_->get_def_use_mgr()->AnalyzeInstUse(&*phi_inst);
   }
@@ -562,7 +625,8 @@ void SSARewriter::FinalizePhiCandidate(PhiCandidate* phi_candidate) {
   // This candidate is now completed.
   phi_candidate->MarkComplete();
 
-  // If |phi_candidate| is not trivial, add it to the list of Phis to generate.
+  // If |phi_candidate| is not trivial, add it to the list of Phis to
+  // generate.
   if (TryRemoveTrivialPhi(phi_candidate) == phi_candidate->result_id()) {
     // If we could not remove |phi_candidate|, it means that it is complete
     // and not trivial. Add it to the list of Phis to generate.

source/val/validate_builtins.cpp

@@ -2603,17 +2603,19 @@ spv_result_t BuiltInsValidator::ValidateLayerOrViewportIndexAtReference(
       assert(function_id_ == 0);
       for (const auto em :
            {SpvExecutionModelVertex, SpvExecutionModelTessellationEvaluation,
-            SpvExecutionModelGeometry}) {
+            SpvExecutionModelGeometry, SpvExecutionModelMeshNV}) {
         id_to_at_reference_checks_[referenced_from_inst.id()].push_back(
-            std::bind(&BuiltInsValidator::ValidateNotCalledWithExecutionModel,
-                      this,
-                      "Vulkan spec doesn't allow BuiltIn Layer and "
-                      "ViewportIndex to be "
-                      "used for variables with Input storage class if "
-                      "execution model is Vertex, TessellationEvaluation, or "
-                      "Geometry.",
-                      em, decoration, built_in_inst, referenced_from_inst,
-                      std::placeholders::_1));
+            std::bind(
+                &BuiltInsValidator::ValidateNotCalledWithExecutionModel, this,
+                std::string(
+                    _.VkErrorID((operand == SpvBuiltInLayer) ? 4274 : 4406) +
+                    "Vulkan spec doesn't allow BuiltIn Layer and "
+                    "ViewportIndex to be "
+                    "used for variables with Input storage class if "
+                    "execution model is Vertex, TessellationEvaluation, "
+                    "Geometry, or MeshNV."),
+                em, decoration, built_in_inst, referenced_from_inst,
+                std::placeholders::_1));
       }
     }
 
@@ -2621,11 +2623,12 @@ spv_result_t BuiltInsValidator::ValidateLayerOrViewportIndexAtReference(
       assert(function_id_ == 0);
       id_to_at_reference_checks_[referenced_from_inst.id()].push_back(std::bind(
           &BuiltInsValidator::ValidateNotCalledWithExecutionModel, this,
-          "Vulkan spec doesn't allow BuiltIn Layer and "
-          "ViewportIndex to be "
-          "used for variables with Output storage class if "
-          "execution model is "
-          "Fragment.",
+          std::string(_.VkErrorID((operand == SpvBuiltInLayer) ? 4275 : 4407) +
+                      "Vulkan spec doesn't allow BuiltIn Layer and "
+                      "ViewportIndex to be "
+                      "used for variables with Output storage class if "
+                      "execution model is "
+                      "Fragment."),
           SpvExecutionModelFragment, decoration, built_in_inst,
           referenced_from_inst, std::placeholders::_1));
     }

source/val/validation_state.cpp

@@ -1377,6 +1377,10 @@ std::string ValidationState_t::VkErrorID(uint32_t id,
       return VUID_WRAP(VUID-InstanceIndex-InstanceIndex-04265);
     case 4272:
       return VUID_WRAP(VUID-Layer-Layer-04272);
+    case 4274:
+      return VUID_WRAP(VUID-Layer-Layer-04274);
+    case 4275:
+      return VUID_WRAP(VUID-Layer-Layer-04275);
     case 4276:
       return VUID_WRAP(VUID-Layer-Layer-04276);
     case 4281:
@@ -1469,6 +1473,10 @@ std::string ValidationState_t::VkErrorID(uint32_t id,
       return VUID_WRAP(VUID-ViewIndex-ViewIndex-04403);
     case 4404:
       return VUID_WRAP(VUID-ViewportIndex-ViewportIndex-04404);
+    case 4406:
+      return VUID_WRAP(VUID-ViewportIndex-ViewportIndex-04406);
+    case 4407:
+      return VUID_WRAP(VUID-ViewportIndex-ViewportIndex-04407);
     case 4408:
       return VUID_WRAP(VUID-ViewportIndex-ViewportIndex-04408);
     case 4422:
@@ -1483,6 +1491,18 @@ std::string ValidationState_t::VkErrorID(uint32_t id,
       return VUID_WRAP(VUID-WorkgroupSize-WorkgroupSize-04426);
     case 4427:
       return VUID_WRAP(VUID-WorkgroupSize-WorkgroupSize-04427);
+    case 4484:
+      return VUID_WRAP(VUID-PrimitiveShadingRateKHR-PrimitiveShadingRateKHR-04484);
+    case 4485:
+      return VUID_WRAP(VUID-PrimitiveShadingRateKHR-PrimitiveShadingRateKHR-04485);
+    case 4486:
+      return VUID_WRAP(VUID-PrimitiveShadingRateKHR-PrimitiveShadingRateKHR-04486);
+    case 4490:
+      return VUID_WRAP(VUID-ShadingRateKHR-ShadingRateKHR-04490);
+    case 4491:
+      return VUID_WRAP(VUID-ShadingRateKHR-ShadingRateKHR-04491);
+    case 4492:
+      return VUID_WRAP(VUID-ShadingRateKHR-ShadingRateKHR-04492);
     default:
       return "";  // unknown id
   };

test/fuzz/transformation_flatten_conditional_branch_test.cpp

@@ -2041,6 +2041,100 @@ TEST(TransformationFlattenConditionalBranchTest,
   ASSERT_TRUE(IsEqual(env, expected, context.get()));
 }
 
+TEST(TransformationFlattenConditionalBranchTest, ContainsDeadBlocksTest) {
+  std::string shader = R"(
+               OpCapability Shader
+          %1 = OpExtInstImport "GLSL.std.450"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Fragment %4 "main"
+               OpExecutionMode %4 OriginUpperLeft
+               OpSource ESSL 320
+               OpName %4 "main"
+          %2 = OpTypeVoid
+          %3 = OpTypeFunction %2
+          %6 = OpTypeBool
+          %7 = OpConstantFalse %6
+          %4 = OpFunction %2 None %3
+          %5 = OpLabel
+               OpSelectionMerge %9 None
+               OpBranchConditional %7 %8 %9
+          %8 = OpLabel
+         %10 = OpCopyObject %6 %7
+               OpBranch %9
+          %9 = OpLabel
+         %11 = OpPhi %6 %10 %8 %7 %5
+         %12 = OpPhi %6 %7 %5 %10 %8
+               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);
+
+  TransformationFlattenConditionalBranch transformation(5, true, 0, 0, 0, {});
+  ASSERT_TRUE(
+      transformation.IsApplicable(context.get(), transformation_context));
+
+  transformation_context.GetFactManager()->AddFactBlockIsDead(8);
+
+  ASSERT_FALSE(
+      transformation.IsApplicable(context.get(), transformation_context));
+}
+
+TEST(TransformationFlattenConditionalBranchTest, ContainsContinueBlockTest) {
+  std::string shader = R"(
+               OpCapability Shader
+          %1 = OpExtInstImport "GLSL.std.450"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Fragment %4 "main"
+               OpExecutionMode %4 OriginUpperLeft
+               OpSource ESSL 320
+               OpName %4 "main"
+          %2 = OpTypeVoid
+          %3 = OpTypeFunction %2
+          %6 = OpTypeBool
+          %7 = OpConstantFalse %6
+          %4 = OpFunction %2 None %3
+         %12 = OpLabel
+               OpBranch %13
+         %13 = OpLabel
+               OpLoopMerge %15 %14 None
+               OpBranchConditional %7 %5 %15
+          %5 = OpLabel
+               OpSelectionMerge %11 None
+               OpBranchConditional %7 %9 %10
+          %9 = OpLabel
+               OpBranch %11
+         %10 = OpLabel
+               OpBranch %14
+         %11 = OpLabel
+               OpBranch %14
+         %14 = OpLabel
+               OpBranch %13
+         %15 = 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);
+
+  ASSERT_FALSE(TransformationFlattenConditionalBranch(5, true, 0, 0, 0, {})
+                   .IsApplicable(context.get(), transformation_context));
+}
+
 }  // namespace
 }  // namespace fuzz
 }  // namespace spvtools

test/opt/local_ssa_elim_test.cpp

@@ -3891,6 +3891,44 @@ TEST_F(LocalSSAElimTest, RemoveDebugDeclareWithoutLoads) {
   SinglePassRunAndMatch<SSARewritePass>(text, true);
 }
 
+// Check support for pointer variables. When pointer variables are used, the
+// computation of reaching definitions may need to follow pointer chains.
+// See https://github.com/KhronosGroup/SPIRV-Tools/issues/3873 for details.
+TEST_F(LocalSSAElimTest, PointerVariables) {
+  const std::string text = R"(
+               OpCapability Shader
+               OpCapability VariablePointers
+               OpExtension "SPV_KHR_variable_pointers"
+               OpMemoryModel Logical Simple
+               OpEntryPoint Fragment %1 "main" %2 %3
+               OpExecutionMode %1 OriginUpperLeft
+      %float = OpTypeFloat 32
+       %void = OpTypeVoid
+          %6 = OpTypeFunction %void
+%_ptr_Input_float = OpTypePointer Input %float
+%_ptr_Output_float = OpTypePointer Output %float
+%_ptr_Function__ptr_Input_float = OpTypePointer Function %_ptr_Input_float
+          %2 = OpVariable %_ptr_Input_float Input
+          %3 = OpVariable %_ptr_Output_float Output
+          %1 = OpFunction %void None %6
+         %10 = OpLabel
+         %11 = OpVariable %_ptr_Function__ptr_Input_float Function
+               OpStore %11 %2
+
+; CHECK-NOT: %12 = OpLoad %_ptr_Input_float %11
+         %12 = OpLoad %_ptr_Input_float %11
+
+; CHECK: %13 = OpLoad %float %2
+         %13 = OpLoad %float %12
+
+               OpStore %3 %13
+               OpReturn
+               OpFunctionEnd
+)";
+
+  SinglePassRunAndMatch<SSARewritePass>(text, true);
+}
+
 // TODO(greg-lunarg): Add tests to verify handling of these cases:
 //
 //    No optimization in the presence of

test/tools/opt/flags.py

@@ -149,6 +149,7 @@ class TestPerformanceOptimizationPasses(expect.ValidObjectFile1_5,
       'eliminate-dead-branches',
       'merge-return',
       'inline-entry-points-exhaustive',
+      'eliminate-dead-functions',
       'eliminate-dead-code-aggressive',
       'private-to-local',
       'eliminate-local-single-block',

test/val/val_builtins_test.cpp

@@ -168,6 +168,17 @@ MATCHER_P(AnyVUID, vuid_set, "VUID from the set is in error message") {
   std::string token;
   std::string vuids = std::string(vuid_set);
   size_t position;
+
+  // Catch case were someone accidentally left spaces by trimming string
+  // clang-format off
+  vuids.erase(std::find_if(vuids.rbegin(), vuids.rend(), [](unsigned char c) {
+    return (c != ' ');
+  }).base(), vuids.end());
+  vuids.erase(vuids.begin(), std::find_if(vuids.begin(), vuids.end(), [](unsigned char c) {
+    return (c != ' ');
+  }));
+  // clang-format on
+
   do {
     position = vuids.find(delimiter);
     if (position != std::string::npos) {
@@ -1263,7 +1274,8 @@ INSTANTIATE_TEST_SUITE_P(
     ValidateVulkanCombineBuiltInExecutionModelDataTypeResult,
     Combine(
         Values("Layer", "ViewportIndex"), Values("Fragment"), Values("Output"),
-        Values("%u32"), Values(nullptr),
+        Values("%u32"),
+        Values("VUID-Layer-Layer-04275 VUID-ViewportIndex-ViewportIndex-04407"),
         Values(TestResult(SPV_ERROR_INVALID_DATA,
                           "Output storage class if execution model is Fragment",
                           "which is called with execution model Fragment"))));
@@ -1274,10 +1286,11 @@ INSTANTIATE_TEST_SUITE_P(
     Combine(
         Values("Layer", "ViewportIndex"),
         Values("Vertex", "TessellationEvaluation", "Geometry"), Values("Input"),
-        Values("%u32"), Values(nullptr),
+        Values("%u32"),
+        Values("VUID-Layer-Layer-04274 VUID-ViewportIndex-ViewportIndex-04406"),
         Values(TestResult(SPV_ERROR_INVALID_DATA,
                           "Input storage class if execution model is Vertex, "
-                          "TessellationEvaluation, or Geometry",
+                          "TessellationEvaluation, Geometry, or MeshNV",
                           "which is called with execution model"))));
 
 INSTANTIATE_TEST_SUITE_P(
@@ -3836,7 +3849,7 @@ INSTANTIATE_TEST_SUITE_P(
         Values("PrimitiveShadingRateKHR"), Values("Vertex"), Values("Output"),
         Values("%f32"), Values("OpCapability FragmentShadingRateKHR\n"),
         Values("OpExtension \"SPV_KHR_fragment_shading_rate\"\n"),
-        Values("VUID-PrimitiveShadingRateKHR-PrimitiveShadingRateKHR-04485 "),
+        Values("VUID-PrimitiveShadingRateKHR-PrimitiveShadingRateKHR-04486 "),
         Values(TestResult(
             SPV_ERROR_INVALID_DATA,
             "According to the Vulkan spec BuiltIn PrimitiveShadingRateKHR "