Merge pull request #41297 from eeckstein/bisecting-improvements

SILPassManager: improve bisecting for debugging the optimizer

Author: eeckstein

docs/DebuggingTheCompiler.md

@@ -714,39 +714,31 @@ which causes the miscompile.
 Currently there is no tool to automatically identify the bad optimization, but
 it's quite easy to do this manually:
 
-1. Find the offending optimization with bisecting:
-
-  a. Add the compiler option `-Xllvm -sil-opt-pass-count=<n>`, where `<n>`
-     is the number of optimizations to run.
-
-  b. Bisect: find n where the executable crashes, but does not crash
-     with n-1. First just try n = 10, 100, 1000, 10000, etc. to find
-     an upper bound). Then can either bisect the invocation by hand or
-     place the invocation into a script and use
-     `./llvm-project/llvm/utils/bisect` to automatically bisect
-     based on the scripts error code. Example invocation:
-
-       bisect --start=0 --end=10000 ./invoke_swift_passing_N.sh "%(count)s"
-
-  c. Once one finds `n`, Add another option `-Xllvm -sil-print-pass-name`. The output can be
-     large, so it's best to redirect stderr to a file (`2> output`).
-     In the output search for the last pass before `stage Address Lowering`.
-     It should be the `Run #<n-1>`. This line tells you the name of the bad
-     optimization pass and on which function it run.
-
-2. Get the SIL before and after the bad optimization.
-
-  a. Add the compiler option
-     `-Xllvm -sil-print-function='<function>'`
-     where `<function>` is the function name (including the preceding `$`).
-     For example:
-     `-Xllvm -sil-print-function='$s4test6testityS2iF'`.
-     Again, the output can be large, so it's best to redirect stderr to a file.
-  b. From the output, copy the SIL of the function *before* the bad
-     run into a separate file and the SIL *after* the bad run into a file.
-  c. Compare both SIL files and try to figure out what the optimization pass
-     did wrong. To simplify the comparison, it's sometimes helpful to replace
-     all SIL values (e.g. `%27`) with a constant string (e.g. `%x`).
+1. Add the compiler option `-Xllvm -sil-opt-pass-count=<n>`, where `<n>`
+   is the number of optimizations to run.
+
+2. Bisect: find n where the executable crashes, but does not crash
+   with n-1. First just try n = 10, 100, 1000, 10000, etc. to find
+   an upper bound). Then can either bisect the invocation by hand or
+   place the invocation into a script and use
+   `./llvm-project/llvm/utils/bisect` to automatically bisect
+   based on the scripts error code. Example invocation:
+
+     bisect --start=0 --end=10000 ./invoke_swift_passing_N.sh "%(count)s"
+
+3. Add another option `-Xllvm -sil-print-last`. The output can be
+   large, so it's best to redirect stderr to a file (`2> output`).
+   The output contains the SIL before and after the bad optimization.
+
+4. Copy the two functions from the output into separate files and
+   compare both files. Try to figure out what the optimization pass
+   did wrong. To simplify the comparison, it's sometimes helpful to replace
+   all SIL values (e.g. `%27`) with a constant string (e.g. `%x`).
+
+5. If the bad optimization is SILCombine or SimplifyCFG (which do a lot of
+   transformations in a single run) it's helpful to continue bisecting on
+   the sub-pass number. The option `-Xllvm -sil-opt-pass-count=<n>.<m>`
+   can be used for that, where `m` is the sub-pass number.
 
 ### Using git-bisect in the presence of branch forwarding/feature branches
 

include/swift/SILOptimizer/PassManager/PassManager.h

@@ -143,6 +143,10 @@ class SILPassManager {
 
   /// The number of passes run so far.
   unsigned NumPassesRun = 0;
+  unsigned numSubpassesRun = 0;
+
+  unsigned maxNumPassesToRun = UINT_MAX;
+  unsigned maxNumSubpassesToRun = UINT_MAX;
 
   /// For invoking Swift passes.
   SwiftPassInvocation swiftPassInvocation;
@@ -352,10 +356,17 @@ class SILPassManager {
 
   void executePassPipelinePlan(const SILPassPipelinePlan &Plan);
 
+  bool continueWithNextSubpassRun(SILInstruction *forInst, SILFunction *function,
+                                  SILTransform *trans);
+
   static bool isPassDisabled(StringRef passName);
   static bool disablePassesForFunction(SILFunction *function);
 
 private:
+  bool doPrintBefore(SILTransform *T, SILFunction *F);
+
+  bool doPrintAfter(SILTransform *T, SILFunction *F, bool PassChangedSIL);
+
   void execute();
 
   /// Add a pass of a specific kind.
@@ -387,7 +398,8 @@ class SILPassManager {
   bool analysesUnlocked();
 
   /// Dumps information about the pass with index \p TransIdx to llvm::dbgs().
-  void dumpPassInfo(const char *Title, SILTransform *Tr, SILFunction *F);
+  void dumpPassInfo(const char *Title, SILTransform *Tr, SILFunction *F,
+                    int passIdx = -1);
 
   /// Dumps information about the pass with index \p TransIdx to llvm::dbgs().
   void dumpPassInfo(const char *Title, unsigned TransIdx,

include/swift/SILOptimizer/PassManager/Transforms.h

@@ -19,6 +19,7 @@
 namespace swift {
   class SILModule;
   class SILFunction;
+  class SILInstruction;
   class PrettyStackTraceSILFunctionTransform;
 
   /// The base class for all SIL-level transformations.
@@ -129,6 +130,10 @@ namespace swift {
 
     SILFunction *getFunction() { return F; }
 
+    bool continueWithNextSubpassRun(SILInstruction *forInst = nullptr) {
+      return PM->continueWithNextSubpassRun(forInst, F, this);
+    }
+
     void invalidateAnalysis(SILAnalysis::InvalidationKind K) {
       PM->invalidateAnalysis(F, K);
     }

lib/SILOptimizer/PassManager/PassManager.cpp

@@ -52,9 +52,13 @@ llvm::cl::opt<bool> SILPrintPassTime(
     "sil-print-pass-time", llvm::cl::init(false),
     llvm::cl::desc("Print the execution time of each SIL pass"));
 
-llvm::cl::opt<unsigned> SILNumOptPassesToRun(
-    "sil-opt-pass-count", llvm::cl::init(UINT_MAX),
-    llvm::cl::desc("Stop optimizing after <N> optimization passes"));
+llvm::cl::opt<bool> SILPrintLast(
+    "sil-print-last", llvm::cl::init(false),
+    llvm::cl::desc("Print the last optimized function before and after the last pass"));
+
+llvm::cl::opt<std::string> SILNumOptPassesToRun(
+    "sil-opt-pass-count", llvm::cl::init(""),
+    llvm::cl::desc("Stop optimizing after <N> passes or <N>.<M> passes/sub-passes"));
 
 llvm::cl::opt<std::string> SILBreakOnFun(
     "sil-break-on-function", llvm::cl::init(""),
@@ -178,7 +182,11 @@ static bool functionSelectionEmpty() {
   return SILPrintFunction.empty() && SILPrintFunctions.empty();
 }
 
-static bool doPrintBefore(SILTransform *T, SILFunction *F) {
+bool SILPassManager::doPrintBefore(SILTransform *T, SILFunction *F) {
+  if (NumPassesRun == maxNumPassesToRun - 1 && SILPrintLast &&
+      maxNumSubpassesToRun == UINT_MAX && !isMandatory)
+    return true;
+
   if (F && !isFunctionSelectedForPrinting(F))
     return false;
 
@@ -201,7 +209,10 @@ static bool doPrintBefore(SILTransform *T, SILFunction *F) {
   return false;
 }
 
-static bool doPrintAfter(SILTransform *T, SILFunction *F, bool PassChangedSIL) {
+bool SILPassManager::doPrintAfter(SILTransform *T, SILFunction *F, bool PassChangedSIL) {
+  if (NumPassesRun == maxNumPassesToRun - 1 && SILPrintLast && !isMandatory)
+    return true;
+
   if (F && !isFunctionSelectedForPrinting(F))
     return false;
 
@@ -316,6 +327,22 @@ SILPassManager::SILPassManager(SILModule *M, bool isMandatory,
   Analyses.push_back(create##NAME##Analysis(Mod));
 #include "swift/SILOptimizer/Analysis/Analysis.def"
 
+  if (!SILNumOptPassesToRun.empty()) {
+    StringRef countsStr = SILNumOptPassesToRun;
+    bool validFormat = true;
+    if (countsStr.consumeInteger(10, maxNumPassesToRun))
+      validFormat = false;
+    if (countsStr.startswith(".")) {
+      countsStr = countsStr.drop_front(1);
+      if (countsStr.consumeInteger(10, maxNumSubpassesToRun))
+        validFormat = false;
+    }
+    if (!validFormat || !countsStr.empty()) {
+      llvm::errs() << "error: wrong format of -sil-opt-pass-count option\n";
+      exit(1);
+    }
+  }
+
   for (SILAnalysis *A : Analyses) {
     A->initialize(this);
   }
@@ -329,7 +356,27 @@ SILPassManager::SILPassManager(SILModule *M, bool isMandatory,
 bool SILPassManager::continueTransforming() {
   if (isMandatory)
     return true;
-  return NumPassesRun < SILNumOptPassesToRun;
+  return NumPassesRun < maxNumPassesToRun;
+}
+
+bool SILPassManager::continueWithNextSubpassRun(SILInstruction *forInst,
+                                                SILFunction *function,
+                                                SILTransform *trans) {
+  if (isMandatory)
+    return true;
+  if (NumPassesRun != maxNumPassesToRun - 1)
+    return true;
+
+  unsigned subPass = numSubpassesRun++;
+  
+  if (subPass == maxNumSubpassesToRun - 1 && SILPrintLast) {
+    dumpPassInfo("*** SIL function before ", trans, function);
+    if (forInst) {
+      llvm::dbgs() << "  *** sub-pass " << subPass << " for " << *forInst;
+    }
+    function->dump(getOptions().EmitVerboseSIL);
+  }
+  return subPass < maxNumSubpassesToRun;
 }
 
 bool SILPassManager::analysesUnlocked() {
@@ -359,24 +406,20 @@ static bool breakBeforeRunning(StringRef fnName, SILFunctionTransform *SFT) {
 }
 
 void SILPassManager::dumpPassInfo(const char *Title, SILTransform *Tr,
-                                  SILFunction *F) {
-  llvm::dbgs() << "  " << Title << " #" << NumPassesRun << ", stage "
-               << StageName << ", pass : " << Tr->getID()
-               << " (" << Tr->getTag() << ")";
+                                  SILFunction *F, int passIdx) {
+  llvm::dbgs() << "  " << Title << " #" << NumPassesRun
+               << ", stage " << StageName << ", pass";
+  if (passIdx >= 0)
+    llvm::dbgs() << ' ' << passIdx;
+  llvm::dbgs() << ": " << Tr->getID() << " (" << Tr->getTag() << ")";
   if (F)
     llvm::dbgs() << ", Function: " << F->getName();
   llvm::dbgs() << '\n';
 }
 
 void SILPassManager::dumpPassInfo(const char *Title, unsigned TransIdx,
                                   SILFunction *F) {
-  SILTransform *Tr = Transformations[TransIdx];
-  llvm::dbgs() << "  " << Title << " #" << NumPassesRun << ", stage "
-    << StageName << ", pass " << TransIdx << ": " << Tr->getID()
-    << " (" << Tr->getTag() << ")";
-  if (F)
-    llvm::dbgs() << ", Function: " << F->getName();
-  llvm::dbgs() << '\n';
+  dumpPassInfo(Title, Transformations[TransIdx], F, (int)TransIdx);
 }
 
 bool SILPassManager::isMandatoryFunctionPass(SILFunctionTransform *sft) {
@@ -451,6 +494,7 @@ void SILPassManager::runPassOnFunction(unsigned TransIdx, SILFunction *F) {
   updateSILModuleStatsBeforeTransform(F->getModule(), SFT, *this, NumPassesRun);
 
   CurrentPassHasInvalidated = false;
+  numSubpassesRun = 0;
 
   auto MatchFun = [&](const std::string &Str) -> bool {
     return SFT->getTag().contains(Str) || SFT->getID().contains(Str);
@@ -623,6 +667,7 @@ void SILPassManager::runModulePass(unsigned TransIdx) {
   updateSILModuleStatsBeforeTransform(*Mod, SMT, *this, NumPassesRun);
 
   CurrentPassHasInvalidated = false;
+  numSubpassesRun = 0;
 
   if (SILPrintPassName)
     dumpPassInfo("Run module pass", TransIdx);

lib/SILOptimizer/SILCombiner/SILCombine.cpp

@@ -161,6 +161,57 @@ class SILCombineCanonicalize final : CanonicalizeInstruction {
   }
 };
 
+SILCombiner::SILCombiner(SILFunctionTransform *trans,
+                         bool removeCondFails, bool enableCopyPropagation) :
+  parentTransform(trans),
+  AA(trans->getPassManager()->getAnalysis<AliasAnalysis>(trans->getFunction())),
+  DA(trans->getPassManager()->getAnalysis<DominanceAnalysis>()),
+  PCA(trans->getPassManager()->getAnalysis<ProtocolConformanceAnalysis>()),
+  CHA(trans->getPassManager()->getAnalysis<ClassHierarchyAnalysis>()),
+  NLABA(trans->getPassManager()->getAnalysis<NonLocalAccessBlockAnalysis>()),
+  Worklist("SC"),
+  deleter(InstModCallbacks()
+              .onDelete([&](SILInstruction *instToDelete) {
+                // We allow for users in SILCombine to perform 2 stage
+                // deletion, so we need to split the erasing of
+                // instructions from adding operands to the worklist.
+                eraseInstFromFunction(*instToDelete,
+                                      false /* don't add operands */);
+              })
+              .onNotifyWillBeDeleted(
+                  [&](SILInstruction *instThatWillBeDeleted) {
+                    Worklist.addOperandsToWorklist(
+                      *instThatWillBeDeleted);
+                  })
+              .onCreateNewInst([&](SILInstruction *newlyCreatedInst) {
+                Worklist.add(newlyCreatedInst);
+              })
+              .onSetUseValue([&](Operand *use, SILValue newValue) {
+                use->set(newValue);
+                Worklist.add(use->getUser());
+              })),
+  deadEndBlocks(trans->getFunction()), MadeChange(false),
+  RemoveCondFails(removeCondFails),
+  enableCopyPropagation(enableCopyPropagation), Iteration(0),
+  Builder(*trans->getFunction(), &TrackingList),
+  FuncBuilder(*trans),
+  CastOpt(
+      FuncBuilder, nullptr /*SILBuilderContext*/,
+      /* ReplaceValueUsesAction */
+      [&](SILValue Original, SILValue Replacement) {
+        replaceValueUsesWith(Original, Replacement);
+      },
+      /* ReplaceInstUsesAction */
+      [&](SingleValueInstruction *I, ValueBase *V) {
+        replaceInstUsesWith(*I, V);
+      },
+      /* EraseAction */
+      [&](SILInstruction *I) { eraseInstFromFunction(*I); }),
+  deBlocks(trans->getFunction()),
+  ownershipFixupContext(getInstModCallbacks(), deBlocks),
+  swiftPassInvocation(trans->getPassManager(),
+                      trans->getFunction(), this) {}
+
 bool SILCombiner::trySinkOwnedForwardingInst(SingleValueInstruction *svi) {
   if (auto *consumingUse = svi->getSingleConsumingUse()) {
     auto *consumingUser = consumingUse->getUser();
@@ -355,6 +406,9 @@ bool SILCombiner::doOneIteration(SILFunction &F, unsigned Iteration) {
     if (I == nullptr)
       continue;
 
+    if (!parentTransform->continueWithNextSubpassRun(I))
+      return false;
+
     // Check to see if we can DCE the instruction.
     if (isInstructionTriviallyDead(I)) {
       LLVM_DEBUG(llvm::dbgs() << "SC: DCE: " << *I << '\n');
@@ -455,7 +509,7 @@ bool SILCombiner::runOnFunction(SILFunction &F) {
     StackNesting::fixNesting(&F);
   }
 
-  // Cleanup the builder and return whether or not we made any changes.
+  assert(TrackingList.empty() && "TrackingList should be fully processed");
   return Changed;
 }
 
@@ -532,33 +586,18 @@ namespace {
 
 class SILCombine : public SILFunctionTransform {
 
-  llvm::SmallVector<SILInstruction *, 64> TrackingList;
-  
   /// The entry point to the transformation.
   void run() override {
-    auto *AA = PM->getAnalysis<AliasAnalysis>(getFunction());
-    auto *DA = PM->getAnalysis<DominanceAnalysis>();
-    auto *PCA = PM->getAnalysis<ProtocolConformanceAnalysis>();
-    auto *CHA = PM->getAnalysis<ClassHierarchyAnalysis>();
-    auto *NLABA = PM->getAnalysis<NonLocalAccessBlockAnalysis>();
-
     bool enableCopyPropagation =
         getOptions().CopyPropagation == CopyPropagationOption::On;
     if (getOptions().EnableOSSAModules) {
       enableCopyPropagation =
           getOptions().CopyPropagation != CopyPropagationOption::Off;
     }
 
-    SILOptFunctionBuilder FuncBuilder(*this);
-    // Create a SILBuilder with a tracking list for newly added
-    // instructions, which we will periodically move to our worklist.
-    SILBuilder B(*getFunction(), &TrackingList);
-    SILCombiner Combiner(this, FuncBuilder, B, AA, DA, PCA, CHA, NLABA,
-                         getOptions().RemoveRuntimeAsserts,
+    SILCombiner Combiner(this, getOptions().RemoveRuntimeAsserts,
                          enableCopyPropagation);
     bool Changed = Combiner.runOnFunction(*getFunction());
-    assert(TrackingList.empty() &&
-           "TrackingList should be fully processed by SILCombiner");
 
     if (Changed) {
       // Invalidate everything.