spirv-fuzz: Only recommend passes when a pass had an effect (#3863)

Fixes #3817.

source/fuzz/fuzzer.cpp

@@ -324,8 +324,9 @@ Fuzzer::FuzzerResult Fuzzer::Run() {
   }
 
   do {
-    if (!ApplyPassAndCheckValidity(repeated_pass_manager->ChoosePass(),
-                                   tools)) {
+    if (!ApplyPassAndCheckValidity(
+            repeated_pass_manager->ChoosePass(transformation_sequence_out_),
+            tools)) {
       return {Fuzzer::FuzzerResultStatus::kFuzzerPassLedToInvalidModule,
               std::vector<uint32_t>(), protobufs::TransformationSequence()};
     }

source/fuzz/pass_management/repeated_pass_manager.h

@@ -18,6 +18,7 @@
 #include "source/fuzz/fuzzer_context.h"
 #include "source/fuzz/fuzzer_pass.h"
 #include "source/fuzz/pass_management/repeated_pass_instances.h"
+#include "source/fuzz/protobufs/spirvfuzz_protobufs.h"
 
 namespace spvtools {
 namespace fuzz {
@@ -33,8 +34,11 @@ class RepeatedPassManager {
 
   virtual ~RepeatedPassManager();
 
-  // Returns the fuzzer pass instance that should be run next.
-  virtual FuzzerPass* ChoosePass() = 0;
+  // Returns the fuzzer pass instance that should be run next.  The
+  // transformations that have been applied so far are provided via
+  // |applied_transformations| and can be used to influence the decision.
+  virtual FuzzerPass* ChoosePass(
+      const protobufs::TransformationSequence& applied_transformations) = 0;
 
  protected:
   FuzzerContext* GetFuzzerContext() { return fuzzer_context_; }

source/fuzz/pass_management/repeated_pass_manager_looped_with_recommendations.cpp

@@ -21,7 +21,9 @@ RepeatedPassManagerLoopedWithRecommendations::
     RepeatedPassManagerLoopedWithRecommendations(
         FuzzerContext* fuzzer_context, RepeatedPassInstances* pass_instances,
         RepeatedPassRecommender* pass_recommender)
-    : RepeatedPassManager(fuzzer_context, pass_instances), next_pass_index_(0) {
+    : RepeatedPassManager(fuzzer_context, pass_instances),
+      num_transformations_applied_before_last_pass_choice_(0),
+      next_pass_index_(0) {
   auto& passes = GetPassInstances()->GetPasses();
   do {
     FuzzerPass* current_pass =
@@ -29,6 +31,8 @@ RepeatedPassManagerLoopedWithRecommendations::
     pass_loop_.push_back(current_pass);
     for (auto future_pass :
          pass_recommender->GetFuturePassRecommendations(*current_pass)) {
+      recommended_pass_indices_.insert(
+          static_cast<uint32_t>(pass_loop_.size()));
       pass_loop_.push_back(future_pass);
     }
   } while (fuzzer_context->ChoosePercentage(
@@ -38,7 +42,24 @@ RepeatedPassManagerLoopedWithRecommendations::
 RepeatedPassManagerLoopedWithRecommendations::
     ~RepeatedPassManagerLoopedWithRecommendations() = default;
 
-FuzzerPass* RepeatedPassManagerLoopedWithRecommendations::ChoosePass() {
+FuzzerPass* RepeatedPassManagerLoopedWithRecommendations::ChoosePass(
+    const protobufs::TransformationSequence& applied_transformations) {
+  assert((next_pass_index_ > 0 ||
+          recommended_pass_indices_.count(next_pass_index_) == 0) &&
+         "The first pass in the loop should not be a recommendation.");
+  assert(static_cast<uint32_t>(applied_transformations.transformation_size()) >=
+             num_transformations_applied_before_last_pass_choice_ &&
+         "The number of applied transformations should not decrease.");
+  if (num_transformations_applied_before_last_pass_choice_ ==
+      static_cast<uint32_t>(applied_transformations.transformation_size())) {
+    // The last pass that was applied did not lead to any new transformations.
+    // We thus do not want to apply recommendations based on it, so we skip on
+    // to the next non-recommended pass.
+    while (recommended_pass_indices_.count(next_pass_index_)) {
+      next_pass_index_ =
+          (next_pass_index_ + 1) % static_cast<uint32_t>(pass_loop_.size());
+    }
+  }
   auto result = pass_loop_[next_pass_index_];
   next_pass_index_ =
       (next_pass_index_ + 1) % static_cast<uint32_t>(pass_loop_.size());

source/fuzz/pass_management/repeated_pass_manager_looped_with_recommendations.h

@@ -41,12 +41,23 @@ class RepeatedPassManagerLoopedWithRecommendations
 
   ~RepeatedPassManagerLoopedWithRecommendations() override;
 
-  FuzzerPass* ChoosePass() override;
+  FuzzerPass* ChoosePass(const protobufs::TransformationSequence&
+                             applied_transformations) override;
 
  private:
   // The loop of fuzzer passes to be applied, populated on construction.
   std::vector<FuzzerPass*> pass_loop_;
 
+  // A set of indices into |pass_loop_| recording which passes are in the loop
+  // because they are recommended based on previous passes in the loop.  This
+  // allows these recommended passes to be skipped if the passes they are
+  // meant to amplify had no effect.
+  std::unordered_set<uint32_t> recommended_pass_indices_;
+
+  // Used to detect when chosen passes have had no effect, so that their
+  // associated recommendations are skipped.
+  uint32_t num_transformations_applied_before_last_pass_choice_;
+
   // An index into |pass_loop_| specifying which pass should be served up next
   // time ChoosePass is invoked.
   uint32_t next_pass_index_;

source/fuzz/pass_management/repeated_pass_manager_random_with_recommendations.cpp

@@ -22,12 +22,29 @@ RepeatedPassManagerRandomWithRecommendations::
         FuzzerContext* fuzzer_context, RepeatedPassInstances* pass_instances,
         RepeatedPassRecommender* pass_recommender)
     : RepeatedPassManager(fuzzer_context, pass_instances),
-      pass_recommender_(pass_recommender) {}
+      pass_recommender_(pass_recommender),
+      num_transformations_applied_before_last_pass_choice_(0),
+      last_pass_choice_(nullptr) {}
 
 RepeatedPassManagerRandomWithRecommendations::
     ~RepeatedPassManagerRandomWithRecommendations() = default;
 
-FuzzerPass* RepeatedPassManagerRandomWithRecommendations::ChoosePass() {
+FuzzerPass* RepeatedPassManagerRandomWithRecommendations::ChoosePass(
+    const protobufs::TransformationSequence& applied_transformations) {
+  assert(static_cast<uint32_t>(applied_transformations.transformation_size()) >=
+             num_transformations_applied_before_last_pass_choice_ &&
+         "The number of applied transformations should not decrease.");
+  if (last_pass_choice_ != nullptr &&
+      static_cast<uint32_t>(applied_transformations.transformation_size()) >
+          num_transformations_applied_before_last_pass_choice_) {
+    // The last pass had some effect, so we make future recommendations based on
+    // it.
+    for (auto future_pass :
+         pass_recommender_->GetFuturePassRecommendations(*last_pass_choice_)) {
+      recommended_passes_.push_back(future_pass);
+    }
+  }
+
   FuzzerPass* result;
   if (recommended_passes_.empty() || GetFuzzerContext()->ChooseEven()) {
     auto& passes = GetPassInstances()->GetPasses();
@@ -36,10 +53,6 @@ FuzzerPass* RepeatedPassManagerRandomWithRecommendations::ChoosePass() {
     result = recommended_passes_.front();
     recommended_passes_.pop_front();
   }
-  for (auto future_pass :
-       pass_recommender_->GetFuturePassRecommendations(*result)) {
-    recommended_passes_.push_back(future_pass);
-  }
   return result;
 }
 

source/fuzz/pass_management/repeated_pass_manager_random_with_recommendations.h

@@ -42,7 +42,8 @@ class RepeatedPassManagerRandomWithRecommendations
 
   ~RepeatedPassManagerRandomWithRecommendations() override;
 
-  FuzzerPass* ChoosePass() override;
+  FuzzerPass* ChoosePass(const protobufs::TransformationSequence&
+                             applied_transformations) override;
 
  private:
   // The queue of passes that have been recommended based on previously-chosen
@@ -51,6 +52,14 @@ class RepeatedPassManagerRandomWithRecommendations
 
   // Used to recommend future passes.
   RepeatedPassRecommender* pass_recommender_;
+
+  // Used to detect when chosen passes have had no effect, so that their
+  // associated recommendations are skipped.
+  uint32_t num_transformations_applied_before_last_pass_choice_;
+
+  // The fuzzer pass returned last time ChoosePass() was called; nullptr if
+  // ChoosePass() has not yet been called.
+  FuzzerPass* last_pass_choice_;
 };
 
 }  // namespace fuzz

source/fuzz/pass_management/repeated_pass_manager_simple.cpp

@@ -23,7 +23,8 @@ RepeatedPassManagerSimple::RepeatedPassManagerSimple(
 
 RepeatedPassManagerSimple::~RepeatedPassManagerSimple() = default;
 
-FuzzerPass* RepeatedPassManagerSimple::ChoosePass() {
+FuzzerPass* RepeatedPassManagerSimple::ChoosePass(
+    const protobufs::TransformationSequence& /*unused*/) {
   auto& passes = GetPassInstances()->GetPasses();
   return passes[GetFuzzerContext()->RandomIndex(passes)].get();
 }

source/fuzz/pass_management/repeated_pass_manager_simple.h

@@ -29,7 +29,8 @@ class RepeatedPassManagerSimple : public RepeatedPassManager {
 
   ~RepeatedPassManagerSimple() override;
 
-  FuzzerPass* ChoosePass() override;
+  FuzzerPass* ChoosePass(const protobufs::TransformationSequence&
+                             applied_transformations) override;
 };
 
 }  // namespace fuzz