Merge pull request #17056 from eeckstein/fix-closure-specializer-4.2

Author: swift-ci

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 : $()
+}