Merge pull request #34084 from meg-gupta/revertomefuncpassandcopyfwd

swift-ci · web-flow · commit 5f92463a0b89 · 2020-09-25T14:18:46.000-07:00
diff --git a/include/swift/SIL/SILCloner.h b/include/swift/SIL/SILCloner.h
@@ -1542,13 +1542,6 @@ template <typename ImplClass>
 void SILCloner<ImplClass>::visitUncheckedValueCastInst(
     UncheckedValueCastInst *Inst) {
   getBuilder().setCurrentDebugScope(getOpScope(Inst->getDebugScope()));
-  if (!getBuilder().hasOwnership()) {
-    recordClonedInstruction(Inst, getBuilder().createUncheckedBitwiseCast(
-                                      getOpLocation(Inst->getLoc()),
-                                      getOpValue(Inst->getOperand()),
-                                      getOpType(Inst->getType())));
-    return;
-  }
   recordClonedInstruction(Inst, getBuilder().createUncheckedValueCast(
                                     getOpLocation(Inst->getLoc()),
                                     getOpValue(Inst->getOperand()),
diff --git a/include/swift/SIL/SILModule.h b/include/swift/SIL/SILModule.h
@@ -263,15 +263,6 @@ class SILModule {
   /// to ensure that the module is serialized only once.
   bool serialized;
 
-  /// Set if we have registered a deserialization notification handler for
-  /// lowering ownership in non transparent functions.
-  /// This gets set in NonTransparent OwnershipModelEliminator pass.
-  bool regDeserializationNotificationHandlerForNonTransparentFuncOME;
-  /// Set if we have registered a deserialization notification handler for
-  /// lowering ownership in transparent functions.
-  /// This gets set in OwnershipModelEliminator pass.
-  bool regDeserializationNotificationHandlerForAllFuncOME;
-
   /// Action to be executed for serializing the SILModule.
   ActionCallback SerializeSILAction;
 
@@ -310,19 +301,6 @@ class SILModule {
     deserializationNotificationHandlers.erase(handler);
   }
 
-  bool hasRegisteredDeserializationNotificationHandlerForNonTransparentFuncOME() {
-    return regDeserializationNotificationHandlerForNonTransparentFuncOME;
-  }
-  bool hasRegisteredDeserializationNotificationHandlerForAllFuncOME() {
-    return regDeserializationNotificationHandlerForAllFuncOME;
-  }
-  void setRegisteredDeserializationNotificationHandlerForNonTransparentFuncOME() {
-    regDeserializationNotificationHandlerForNonTransparentFuncOME = true;
-  }
-  void setRegisteredDeserializationNotificationHandlerForAllFuncOME() {
-    regDeserializationNotificationHandlerForAllFuncOME = true;
-  }
-
   /// Add a delete notification handler \p Handler to the module context.
   void registerDeleteNotificationHandler(DeleteNotificationHandler* Handler);
 
diff --git a/include/swift/SILOptimizer/PassManager/PassManager.h b/include/swift/SILOptimizer/PassManager/PassManager.h
@@ -281,11 +281,6 @@ class SILPassManager {
   /// Run the passes in Transform from \p FromTransIdx to \p ToTransIdx.
   void runFunctionPasses(unsigned FromTransIdx, unsigned ToTransIdx);
 
-  /// Helper function to check if the function pass should be run mandatorily
-  /// All passes in mandatory pass pipeline and ownership model elimination are
-  /// mandatory function passes.
-  bool isMandatoryFunctionPass(SILFunctionTransform *);
-
   /// A helper function that returns (based on SIL stage and debug
   /// options) whether we should continue running passes.
   bool continueTransforming();
diff --git a/lib/SIL/IR/SILModule.cpp b/lib/SIL/IR/SILModule.cpp
@@ -95,8 +95,6 @@ class SILModule::SerializationCallback final
 SILModule::SILModule(llvm::PointerUnion<FileUnit *, ModuleDecl *> context,
                      Lowering::TypeConverter &TC, const SILOptions &Options)
     : Stage(SILStage::Raw), Options(Options), serialized(false),
-      regDeserializationNotificationHandlerForNonTransparentFuncOME(false),
-      regDeserializationNotificationHandlerForAllFuncOME(false),
       SerializeSILAction(), Types(TC) {
   assert(!context.isNull());
   if (auto *file = context.dyn_cast<FileUnit *>()) {
diff --git a/lib/SILOptimizer/Mandatory/OwnershipModelEliminator.cpp b/lib/SILOptimizer/Mandatory/OwnershipModelEliminator.cpp
@@ -370,7 +370,7 @@ static void prepareSILFunctionForOptimization(ModuleDecl *, SILFunction *F) {
 
 namespace {
 
-struct OwnershipModelEliminator : SILFunctionTransform {
+struct OwnershipModelEliminator : SILModuleTransform {
   bool SkipTransparent;
   bool SkipStdlibModule;
 
@@ -379,50 +379,53 @@ struct OwnershipModelEliminator : SILFunctionTransform {
 
   void run() override {
     if (DumpBefore.size()) {
-      getFunction()->dump(DumpBefore.c_str());
+      getModule()->dump(DumpBefore.c_str());
     }
 
-    auto *F = getFunction();
-    auto &Mod = getFunction()->getModule();
+    auto &Mod = *getModule();
 
     // If we are supposed to skip the stdlib module and we are in the stdlib
     // module bail.
     if (SkipStdlibModule && Mod.isStdlibModule()) {
       return;
     }
 
-    if (!F->hasOwnership())
-      return;
-
-    // If we were asked to not strip ownership from transparent functions in
-    // /our/ module, return.
-    if (SkipTransparent && F->isTransparent())
-      return;
-
-    // Verify here to make sure ownership is correct before we strip.
-    {
-      // Add a pretty stack trace entry to tell users who see a verification
-      // failure triggered by this verification check that they need to re-run
-      // with -sil-verify-all to actually find the pass that introduced the
-      // verification error.
-      //
-      // DISCUSSION: This occurs due to the crash from the verification
-      // failure happening in the pass itself. This causes us to dump the
-      // SILFunction and emit a msg that this pass (OME) is the culprit. This
-      // is generally correct for most passes, but not for OME since we are
-      // verifying before we have even modified the function to ensure that
-      // all ownership invariants have been respected before we lower
-      // ownership from the function.
-      llvm::PrettyStackTraceString silVerifyAllMsgOnFailure(
-          "Found verification error when verifying before lowering "
-          "ownership. Please re-run with -sil-verify-all to identify the "
-          "actual pass that introduced the verification error.");
-      F->verify();
-    }
+    for (auto &F : Mod) {
+      // If F does not have ownership, skip it. We have no further work to do.
+      if (!F.hasOwnership())
+        continue;
+
+      // If we were asked to not strip ownership from transparent functions in
+      // /our/ module, continue.
+      if (SkipTransparent && F.isTransparent())
+        continue;
+
+      // Verify here to make sure ownership is correct before we strip.
+      {
+        // Add a pretty stack trace entry to tell users who see a verification
+        // failure triggered by this verification check that they need to re-run
+        // with -sil-verify-all to actually find the pass that introduced the
+        // verification error.
+        //
+        // DISCUSSION: This occurs due to the crash from the verification
+        // failure happening in the pass itself. This causes us to dump the
+        // SILFunction and emit a msg that this pass (OME) is the culprit. This
+        // is generally correct for most passes, but not for OME since we are
+        // verifying before we have even modified the function to ensure that
+        // all ownership invariants have been respected before we lower
+        // ownership from the function.
+        llvm::PrettyStackTraceString silVerifyAllMsgOnFailure(
+            "Found verification error when verifying before lowering "
+            "ownership. Please re-run with -sil-verify-all to identify the "
+            "actual pass that introduced the verification error.");
+        F.verify();
+      }
 
-    if (stripOwnership(*F)) {
-      auto InvalidKind = SILAnalysis::InvalidationKind::BranchesAndInstructions;
-      invalidateAnalysis(InvalidKind);
+      if (stripOwnership(F)) {
+        auto InvalidKind =
+            SILAnalysis::InvalidationKind::BranchesAndInstructions;
+        invalidateAnalysis(&F, InvalidKind);
+      }
     }
 
     // If we were asked to strip transparent, we are at the beginning of the
@@ -432,19 +435,12 @@ struct OwnershipModelEliminator : SILFunctionTransform {
         FunctionBodyDeserializationNotificationHandler;
     std::unique_ptr<DeserializationNotificationHandler> ptr;
     if (SkipTransparent) {
-      if (!Mod.hasRegisteredDeserializationNotificationHandlerForNonTransparentFuncOME()) {
-        ptr.reset(new NotificationHandlerTy(
-            prepareNonTransparentSILFunctionForOptimization));
-        Mod.registerDeserializationNotificationHandler(std::move(ptr));
-        Mod.setRegisteredDeserializationNotificationHandlerForNonTransparentFuncOME();
-      }
+      ptr.reset(new NotificationHandlerTy(
+          prepareNonTransparentSILFunctionForOptimization));
     } else {
-      if (!Mod.hasRegisteredDeserializationNotificationHandlerForAllFuncOME()) {
-        ptr.reset(new NotificationHandlerTy(prepareSILFunctionForOptimization));
-        Mod.registerDeserializationNotificationHandler(std::move(ptr));
-        Mod.setRegisteredDeserializationNotificationHandlerForAllFuncOME();
-      }
+      ptr.reset(new NotificationHandlerTy(prepareSILFunctionForOptimization));
     }
+    Mod.registerDeserializationNotificationHandler(std::move(ptr));
   }
 };
 
diff --git a/lib/SILOptimizer/PassManager/PassManager.cpp b/lib/SILOptimizer/PassManager/PassManager.cpp
@@ -383,35 +383,21 @@ void SILPassManager::dumpPassInfo(const char *Title, unsigned TransIdx,
   llvm::dbgs() << '\n';
 }
 
-bool SILPassManager::isMandatoryFunctionPass(SILFunctionTransform *sft) {
-  return isMandatory || sft->getPassKind() ==
-             PassKind::NonTransparentFunctionOwnershipModelEliminator ||
-         sft->getPassKind() == PassKind::OwnershipModelEliminator ||
-         sft->getPassKind() ==
-             PassKind::NonStdlibNonTransparentFunctionOwnershipModelEliminator;
-}
-
 void SILPassManager::runPassOnFunction(unsigned TransIdx, SILFunction *F) {
 
   assert(analysesUnlocked() && "Expected all analyses to be unlocked!");
 
   auto *SFT = cast<SILFunctionTransform>(Transformations[TransIdx]);
-
-  if (!F->shouldOptimize() && !isMandatoryFunctionPass(SFT)) {
-    return;
-  }
-
   SFT->injectPassManager(this);
   SFT->injectFunction(F);
 
   PrettyStackTraceSILFunctionTransform X(SFT, NumPassesRun);
   DebugPrintEnabler DebugPrint(NumPassesRun);
 
   // If nothing changed since the last run of this pass, we can skip this
-  // pass if it is not mandatory
+  // pass.
   CompletedPasses &completedPasses = CompletedPassesMap[F];
-  if (!isMandatoryFunctionPass(SFT) &&
-      completedPasses.test((size_t)SFT->getPassKind()) &&
+  if (completedPasses.test((size_t)SFT->getPassKind()) &&
       !SILDisableSkippingPasses) {
     if (SILPrintPassName)
       dumpPassInfo("(Skip)", TransIdx, F);
@@ -527,7 +513,7 @@ runFunctionPasses(unsigned FromTransIdx, unsigned ToTransIdx) {
 
     // Only include functions that are definitions, and which have not
     // been intentionally excluded from optimization.
-    if (F.isDefinition())
+    if (F.isDefinition() && (isMandatory || F.shouldOptimize()))
       FunctionWorklist.push_back(*I);
   }
 
diff --git a/lib/SILOptimizer/PassManager/PassPipeline.cpp b/lib/SILOptimizer/PassManager/PassPipeline.cpp
@@ -283,10 +283,6 @@ void addFunctionPasses(SILPassPipelinePlan &P,
   // splits up copy_addr.
   P.addCopyForwarding();
 
-  // We earlier eliminated ownership if we are not compiling the stdlib. Now
-  // handle the stdlib functions.
-  P.addNonTransparentFunctionOwnershipModelEliminator();
-
   // Optimize copies from a temporary (an "l-value") to a destination.
   P.addTempLValueOpt();
 
@@ -493,7 +489,10 @@ static void addHighLevelFunctionPipeline(SILPassPipelinePlan &P) {
   // FIXME: update EagerSpecializer to be a function pass!
   P.addEagerSpecializer();
 
-  // stdlib ownership model elimination is done within addFunctionPasses
+  // We earlier eliminated ownership if we are not compiling the stdlib. Now
+  // handle the stdlib functions.
+  P.addNonTransparentFunctionOwnershipModelEliminator();
+
   addFunctionPasses(P, OptimizationLevelKind::HighLevel);
 
   addHighLevelLoopOptPasses(P);
diff --git a/test/DebugInfo/dbgvalue-insertpt.swift b/test/DebugInfo/dbgvalue-insertpt.swift
@@ -1,9 +1,19 @@
-// RUN: %target-swift-frontend -g -emit-ir -Xllvm '-sil-inline-never-functions=next' %s | %FileCheck %s
+// RUN: %target-swift-frontend -g -emit-ir %s | %FileCheck %s
 // FIXME: This test should be testing a non-shadow-copied value instead.
 for i in 0 ..< 3 {
   // CHECK: %[[ALLOCA:[0-9]+]] = alloca %TSiSg
   // CHECK: %i.debug = alloca i{{32|64}}
   // CHECK-NEXT: call void @llvm.dbg.declare(metadata i{{32|64}}* %i.debug,
   // CHECK-SAME:                           metadata ![[I:[0-9]+]],
+  // CHECK: call swiftcc{{.*}} @{{.*}}next{{.*}}
+  // CHECK: %[[LD:[0-9]+]] = load i{{32|64}}, i{{32|64}}*
+  // CHECK: br i1 {{%.*}}, label %[[FAIL:.*]], label %[[SUCCESS:.*]],
+  //
+  // CHECK: [[SUCCESS]]:
+  // CHECK: br label %[[NEXT_BB:.*]],
+  //
+  // CHECK: [[NEXT_BB]]:
+  // CHECK: %[[PHI_VAL:.*]] = phi i{{32|64}} [ %[[LD]], %[[SUCCESS]] ]
+  // CHECK: store i{{32|64}} %[[PHI_VAL]], i{{32|64}}* %i.debug
   // CHECK: ![[I]] = !DILocalVariable(name: "i",
 }
diff --git a/test/IRGen/big_types_corner_cases.swift b/test/IRGen/big_types_corner_cases.swift
@@ -204,6 +204,12 @@ public func testGetFunc() {
 // CHECK: [[CALL1:%.*]] = call {{.*}} @__swift_instantiateConcreteTypeFromMangledName({{.*}} @"$sSayy22big_types_corner_cases9BigStructVcSgGMD"
 // CHECK: [[CALL2:%.*]] = call i8** @"$sSayy22big_types_corner_cases9BigStructVcSgGSayxGSlsWl
 // CHECK:  call swiftcc void @"$sSlsE10firstIndex5where0B0QzSgSb7ElementQzKXE_tKF"(%TSq.{{.*}}* noalias nocapture sret %{{[0-9]+}}, i8* bitcast ({{.*}}* @"$s22big_types_corner_cases9BigStruct{{.*}}_TRTA{{(\.ptrauth)?}}" to i8*), %swift.opaque* %{{[0-9]+}}, %swift.type* %{{[0-9]+}}, i8** [[CALL2]]
+
+// CHECK-LABEL: define{{( dllexport)?}}{{( protected)?}} hidden swiftcc void @"$s22big_types_corner_cases7TestBigC5test2yyF"(%T22big_types_corner_cases7TestBigC* swiftself %0)
+// CHECK: [[CALL1:%.*]] = call {{.*}} @__swift_instantiateConcreteTypeFromMangledName({{.*}} @"$sSaySS2ID_y22big_types_corner_cases9BigStructVcSg7handlertGMD"
+// CHECK: [[CALL2:%.*]] = call i8** @"$sSaySS2ID_y22big_types_corner_cases9BigStructVcSg7handlertGSayxGSlsWl"
+// CHECK: call swiftcc void @"$sSlss16IndexingIteratorVyxG0B0RtzrlE04makeB0ACyF"(%Ts16IndexingIteratorV{{.*}}* noalias nocapture sret {{.*}}, %swift.type* [[CALL1]], i8** [[CALL2]], %swift.opaque* noalias nocapture swiftself {{.*}})
+// CHECK: ret void
 class TestBig {
     typealias Handler = (BigStruct) -> Void
 
diff --git a/test/SILOptimizer/copyforward_ossa.sil b/test/SILOptimizer/copyforward_ossa.sil
@@ -265,7 +265,7 @@ sil [ossa] @_TFSq4someU__fMGSqQ__FQ_GSqQ__ : $@convention(thin) <τ_0_0> (@in τ
 
 sil [ossa] @_TFsoi2neU__FTGSqQ__Vs26_OptionalNilComparisonType_Sb : $@convention(thin) <τ_0_0> (@in Optional<τ_0_0>, _OptionalNilComparisonType) -> Bool
 
-// CHECK-LABEL: sil hidden [ossa] @option_init :
+// CHECK-LABEL: sil hidden [ossa] @option_init
 // CHECK: alloc_stack
 // CHECK: alloc_stack
 // CHECK: alloc_stack
diff --git a/test/SILOptimizer/prespecialize.swift b/test/SILOptimizer/prespecialize.swift
@@ -7,6 +7,12 @@
 
 // CHECK-LABEL: sil [noinline] @$s13prespecialize4test_4sizeySaySiGz_SitF
 //
+// function_ref specialized Collection<A where ...>.makeIterator() -> IndexingIterator<A>
+// CHECK: function_ref @$sSlss16IndexingIteratorVyxG0B0RtzrlE04makeB0ACyFSnySiG_Tg5
+//
+// function_ref specialized IndexingIterator.next() -> A.Element?
+// CHECK: function_ref @$ss16IndexingIteratorV4next7ElementQzSgyFSnySiG_Tg5
+//
 // Look for generic specialization <Swift.Int> of Swift.Array.subscript.getter : (Swift.Int) -> A
 // CHECK: function_ref @$sSayxSicigSi_Tg5
 // CHECK: return
