Fix a SIL optimizer verification failure complaining about a not up-to-date dominator tree.

In some cases the ClosureSpecializer did not invalidate the dominator tree, although it changes the CFG.
This could happen if during analysis critical CFG edges are broken, but at the end no functions are specialized.

Author: eeckstein

lib/SILOptimizer/IPO/ClosureSpecializer.cpp

@@ -784,7 +784,7 @@ void ClosureSpecCloner::populateCloned() {
 namespace {
 
 class SILClosureSpecializerTransform : public SILFunctionTransform {
-  void gatherCallSites(SILFunction *Caller,
+  bool gatherCallSites(SILFunction *Caller,
                        llvm::SmallVectorImpl<ClosureInfo*> &ClosureCandidates,
                        llvm::DenseSet<FullApplySite> &MultipleClosureAI);
   bool specialize(SILFunction *Caller,
@@ -839,7 +839,7 @@ void SILClosureSpecializerTransform::run() {
   invalidateAnalysis(SILAnalysis::InvalidationKind::Everything);
 }
 
-void SILClosureSpecializerTransform::gatherCallSites(
+bool SILClosureSpecializerTransform::gatherCallSites(
     SILFunction *Caller,
     llvm::SmallVectorImpl<ClosureInfo*> &ClosureCandidates,
     llvm::DenseSet<FullApplySite> &MultipleClosureAI) {
@@ -848,6 +848,8 @@ void SILClosureSpecializerTransform::gatherCallSites(
   // make sure that we do not handle call sites with multiple closure arguments.
   llvm::DenseSet<FullApplySite> VisitedAI;
 
+  bool CFGChanged = false;
+
   // For each basic block BB in Caller...
   for (auto &BB : *Caller) {
 
@@ -918,7 +920,7 @@ void SILClosureSpecializerTransform::gatherCallSites(
         // We could fix this by inserting new arguments more carefully, or
         // changing how we model dynamic Self altogether.
         if (mayBindDynamicSelf(ApplyCallee))
-          return;
+          return CFGChanged;
 
         // Ok, we know that we can perform the optimization but not whether or
         // not the optimization is profitable. Find the index of the argument
@@ -991,12 +993,15 @@ void SILClosureSpecializerTransform::gatherCallSites(
       }
       if (CInfo) {
         ValueLifetimeAnalysis VLA(CInfo->Closure, UsePoints);
-        VLA.computeFrontier(CInfo->LifetimeFrontier,
-                            ValueLifetimeAnalysis::AllowToModifyCFG);
+        if (!VLA.computeFrontier(CInfo->LifetimeFrontier,
+                                 ValueLifetimeAnalysis::AllowToModifyCFG)) {
+          CFGChanged = true;
+        }
         ClosureCandidates.push_back(CInfo);
       }
     }
   }
+  return CFGChanged;
 }
 
 bool SILClosureSpecializerTransform::specialize(SILFunction *Caller,
@@ -1008,7 +1013,9 @@ bool SILClosureSpecializerTransform::specialize(SILFunction *Caller,
   // ApplyInsts. Check the profitability of specializing the closure argument.
   llvm::SmallVector<ClosureInfo*, 8> ClosureCandidates;
   llvm::DenseSet<FullApplySite> MultipleClosureAI;
-  gatherCallSites(Caller, ClosureCandidates, MultipleClosureAI);
+  if (gatherCallSites(Caller, ClosureCandidates, MultipleClosureAI)) {
+    invalidateAnalysis(SILAnalysis::InvalidationKind::Branches);
+  }
 
   bool Changed = false;
   for (auto *CInfo : ClosureCandidates) {

test/SILOptimizer/closure_specialize_and_cfg.sil

@@ -0,0 +1,49 @@
+// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -sil-verify-without-invalidation -enable-sil-verify-all -loop-unroll -closure-specialize %s
+
+// Test if the ClosureSpecializer correctly invalidates the dominator tree
+// even if there are no functions specialized.
+// The test just checks if the compiler does not crash.
+// First running LoopUnroll creates the dominator tree, which should then be
+// invalidated by the ClosureSpecializer.
+// If this is not done correctly the verification will complain that the
+// dominator tree is not up to date.
+
+import Builtin
+import Swift
+
+sil @closure : $@convention(thin) () -> ()
+
+sil @use_closure : $@convention(thin) (@owned @callee_owned () -> ()) -> ()
+
+sil hidden [noinline] @use_closure2 : $@convention(thin) (@owned @callee_owned () -> (), @owned @callee_owned () -> ()) -> () {
+bb0(%0 : $@callee_owned () -> (), %1 : $@callee_owned () -> ()):
+  %2 = apply %0() : $@callee_owned () -> ()
+  %3 = apply %1() : $@callee_owned () -> ()
+  %4 = tuple ()
+  return %3 : $()
+}
+
+sil @insert_release_in_liferange_exit_block : $@convention(thin) () -> () {
+bb0:
+  %2 = function_ref @closure : $@convention(thin) () -> ()
+  %3 = partial_apply %2() : $@convention(thin) () -> ()
+  %8 = function_ref @use_closure : $@convention(thin) (@owned @callee_owned () -> ()) -> ()
+  %5 = partial_apply %8(%3) : $@convention(thin) (@owned @callee_owned () -> ()) -> ()
+
+  // There is a critical edge from bb0 to bb2 which is broken by ValueLifetimeAnalysis.
+  cond_br undef, bb2, bb1
+
+bb1:
+  strong_retain %3 : $@callee_owned () -> ()
+  strong_retain %3 : $@callee_owned () -> ()
+  %10 = function_ref @use_closure2 : $@convention(thin) (@owned @callee_owned () -> (), @owned @callee_owned () -> ()) -> ()
+
+  // Passing two closures actually prevents closure specialization.
+  %17 = apply %10(%3, %3) : $@convention(thin) (@owned @callee_owned () -> (), @owned @callee_owned () -> ()) -> ()
+  br bb2
+
+bb2:
+  strong_release %5 : $@callee_owned () -> ()
+  %11 = tuple ()
+  return %11 : $()
+}