Merge pull request #36245 from atrick/mandatory-copyprop

Add support for a mandatory-copy-propagation pass.

Author: atrick

include/swift/AST/SILOptions.h

@@ -44,6 +44,16 @@ class SILOptions {
   /// Remove all runtime assertions during optimizations.
   bool RemoveRuntimeAsserts = false;
 
+  /// Force-run SIL copy propagation to shorten object lifetime in whatever
+  /// optimization pipeline is currently used.
+  /// When this is 'false' the pipeline has default behavior.
+  bool EnableCopyPropagation = false;
+
+  /// Disable SIL copy propagation to preserve object lifetime in whatever
+  /// optimization pipeline is currently used.
+  /// When this is 'false' the pipeline has default behavior.
+  bool DisableCopyPropagation = false;
+
   /// Controls whether the SIL ARC optimizations are run.
   bool EnableARCOptimizations = true;
 

include/swift/Option/FrontendOptions.td

@@ -188,6 +188,11 @@ def batch_scan_input_file
 def import_prescan : Flag<["-"], "import-prescan">,
    HelpText<"When performing a dependency scan, only dentify all imports of the main Swift module sources">;
 
+def enable_copy_propagation : Flag<["-"], "enable-copy-propagation">,
+  HelpText<"Run SIL copy propagation to shorten object lifetime.">;
+def disable_copy_propagation : Flag<["-"], "disable-copy-propagation">,
+  HelpText<"Don't run SIL copy propagation to preserve object lifetime.">;
+
 def enable_infer_public_concurrent_value : Flag<["-"], "enable-infer-public-concurrent-value">,
     HelpText<"Enable inference of ConcurrentValue conformances for public structs and enums">;
 

include/swift/SILOptimizer/PassManager/Passes.def

@@ -202,8 +202,8 @@ PASS(GlobalOpt, "global-opt",
      "SIL Global Optimization")
 PASS(GlobalPropertyOpt, "global-property-opt",
      "Global Property Optimization")
-PASS(GuaranteedARCOpts, "guaranteed-arc-opts",
-     "Guaranteed ARC Optimization")
+PASS(MandatoryARCOpts, "mandatory-arc-opts",
+     "Mandatory ARC Optimization")
 PASS(HighLevelCSE, "high-level-cse",
      "Common Subexpression Elimination on High-Level SIL")
 PASS(HighLevelLICM, "high-level-licm",

include/swift/SILOptimizer/Utils/CanonicalOSSALifetime.h

@@ -187,7 +187,7 @@ class CanonicalizeOSSALifetime {
 private:
   /// If true, then debug_value instructions outside of non-debug
   /// liveness may be pruned during canonicalization.
-  bool pruneDebug;
+  bool pruneDebugMode;
 
   NonLocalAccessBlockAnalysis *accessBlockAnalysis;
   // Lazily initialize accessBlocks only when
@@ -237,12 +237,13 @@ class CanonicalizeOSSALifetime {
   CanonicalOSSAConsumeInfo consumes;
 
 public:
-  CanonicalizeOSSALifetime(bool pruneDebug,
+  CanonicalizeOSSALifetime(bool pruneDebugMode,
                            NonLocalAccessBlockAnalysis *accessBlockAnalysis,
                            DominanceAnalysis *dominanceAnalysis,
                            DeadEndBlocks *deBlocks)
-    : pruneDebug(pruneDebug), accessBlockAnalysis(accessBlockAnalysis),
-      dominanceAnalysis(dominanceAnalysis), deBlocks(deBlocks) {}
+      : pruneDebugMode(pruneDebugMode),
+        accessBlockAnalysis(accessBlockAnalysis),
+        dominanceAnalysis(dominanceAnalysis), deBlocks(deBlocks) {}
 
   SILValue getCurrentDef() const { return currentDef; }
 

lib/Frontend/CompilerInvocation.cpp

@@ -1189,6 +1189,8 @@ static bool ParseSILArgs(SILOptions &Opts, ArgList &Args,
   // -Ounchecked might also set removal of runtime asserts (cond_fail).
   Opts.RemoveRuntimeAsserts |= Args.hasArg(OPT_RemoveRuntimeAsserts);
 
+  Opts.EnableCopyPropagation |= Args.hasArg(OPT_enable_copy_propagation);
+  Opts.DisableCopyPropagation |= Args.hasArg(OPT_disable_copy_propagation);
   Opts.EnableARCOptimizations &= !Args.hasArg(OPT_disable_arc_opts);
   Opts.EnableOSSAModules |= Args.hasArg(OPT_enable_ossa_modules);
   Opts.EnableOSSAOptimizations &= !Args.hasArg(OPT_disable_ossa_opts);

lib/SIL/Verifier/MemoryLifetime.cpp

@@ -168,8 +168,13 @@ void MemoryLocations::analyzeLocations(SILFunction *function) {
     case SILArgumentConvention::Indirect_In:
     case SILArgumentConvention::Indirect_In_Constant:
     case SILArgumentConvention::Indirect_In_Guaranteed:
-    case SILArgumentConvention::Indirect_Inout:
     case SILArgumentConvention::Indirect_Out:
+      // These are not SIL addresses under -enable-sil-opaque-values
+      if (!function->getConventions().useLoweredAddresses())
+        break;
+
+      LLVM_FALLTHROUGH;
+    case SILArgumentConvention::Indirect_Inout:
       analyzeLocation(funcArg);
       break;
     default:

lib/SILOptimizer/PassManager/PassPipeline.cpp

@@ -350,7 +350,9 @@ void addFunctionPasses(SILPassPipelinePlan &P,
   if (P.getOptions().EnableOSSAModules) {
     // We earlier eliminated ownership if we are not compiling the stdlib. Now
     // handle the stdlib functions, re-simplifying, eliminating ARC as we do.
-    P.addCopyPropagation();
+    if (!P.getOptions().DisableCopyPropagation) {
+      P.addCopyPropagation();
+    }
     P.addSemanticARCOpts();
   }
 
@@ -372,7 +374,9 @@ void addFunctionPasses(SILPassPipelinePlan &P,
 
   // Clean up Semantic ARC before we perform additional post-inliner opts.
   if (P.getOptions().EnableOSSAModules) {
-    P.addCopyPropagation();
+    if (!P.getOptions().DisableCopyPropagation) {
+      P.addCopyPropagation();
+    }
     P.addSemanticARCOpts();
   }
 
@@ -437,7 +441,9 @@ void addFunctionPasses(SILPassPipelinePlan &P,
 
   // Run a final round of ARC opts when ownership is enabled.
   if (P.getOptions().EnableOSSAModules) {
-    P.addCopyPropagation();
+    if (!P.getOptions().DisableCopyPropagation) {
+      P.addCopyPropagation();
+    }
     P.addSemanticARCOpts();
   }
 }
@@ -471,7 +477,9 @@ static void addPerfEarlyModulePassPipeline(SILPassPipelinePlan &P) {
   // Cleanup after SILGen: remove trivial copies to temporaries.
   P.addTempRValueOpt();
   // Cleanup after SILGen: remove unneeded borrows/copies.
-  P.addCopyPropagation();
+  if (!P.getOptions().DisableCopyPropagation) {
+    P.addCopyPropagation();
+  }
   P.addSemanticARCOpts();
 
   // Devirtualizes differentiability witnesses into functions that reference them.
@@ -785,7 +793,9 @@ SILPassPipelinePlan::getPerformancePassPipeline(const SILOptions &Options) {
   // Run one last copy propagation/semantic arc opts run before serialization/us
   // lowering ownership.
   if (P.getOptions().EnableOSSAModules) {
-    P.addCopyPropagation();
+    if (!P.getOptions().DisableCopyPropagation) {
+      P.addCopyPropagation();
+    }
     P.addSemanticARCOpts();
   }
 
@@ -835,7 +845,13 @@ SILPassPipelinePlan::getOnonePassPipeline(const SILOptions &Options) {
   P.startPipeline("non-Diagnostic Enabling Mandatory Optimizations");
   P.addForEachLoopUnroll();
   P.addMandatoryCombine();
-  P.addGuaranteedARCOpts();
+  if (P.getOptions().EnableCopyPropagation) {
+    // MandatoryCopyPropagation should only be run at -Onone, not -O.
+    P.addMandatoryCopyPropagation();
+  }
+  // TODO: MandatoryARCOpts should be subsumed by CopyPropagation. There should
+  // be no need to run another analysis of copies at -Onone.
+  P.addMandatoryARCOpts();
 
   // First serialize the SIL if we are asked to.
   P.startPipeline("Serialization");

lib/SILOptimizer/SemanticARC/Context.h

@@ -109,7 +109,7 @@ struct LLVM_LIBRARY_VISIBILITY Context {
   /// As an example, we do not do load [copy] optimizations here since they
   /// generally involve more complex analysis, but simple peepholes of
   /// copy_values we /do/ allow.
-  bool onlyGuaranteedOpts;
+  bool onlyMandatoryOpts;
 
   /// Callbacks that we must use to remove or RAUW values.
   InstModCallbacks instModCallbacks;
@@ -119,11 +119,11 @@ struct LLVM_LIBRARY_VISIBILITY Context {
 
   DeadEndBlocks &getDeadEndBlocks() { return deadEndBlocks; }
 
-  Context(SILFunction &fn, DeadEndBlocks &deBlocks, bool onlyGuaranteedOpts,
+  Context(SILFunction &fn, DeadEndBlocks &deBlocks, bool onlyMandatoryOpts,
           InstModCallbacks callbacks)
       : fn(fn), deadEndBlocks(deBlocks), lifetimeFrontier(),
         addressToExhaustiveWriteListCache(constructCacheValue),
-        onlyGuaranteedOpts(onlyGuaranteedOpts), instModCallbacks(callbacks) {}
+        onlyMandatoryOpts(onlyMandatoryOpts), instModCallbacks(callbacks) {}
 
   void verify() const;
 

lib/SILOptimizer/SemanticARC/CopyValueOpts.cpp

@@ -66,12 +66,11 @@ using namespace swift::semanticarc;
 // are within the borrow scope.
 //
 // TODO: This needs a better name.
-//
-// FIXME: CanonicalizeOSSALifetime replaces this once it supports borrows.
 bool SemanticARCOptVisitor::performGuaranteedCopyValueOptimization(
     CopyValueInst *cvi) {
-  // For now, do not run this optimization. This is just to be careful.
-  if (ctx.onlyGuaranteedOpts)
+  // All mandatory copy optimization is handled by CanonicalizeOSSALifetime,
+  // which knows how to preserve lifetimes for debugging.
+  if (ctx.onlyMandatoryOpts)
     return false;
 
   SmallVector<BorrowedValue, 4> borrowScopeIntroducers;
@@ -721,11 +720,11 @@ bool SemanticARCOptVisitor::tryJoiningCopyValueLiveRangeWithOperand(
 
 /// Given an owned value that is completely enclosed within its parent owned
 /// value and is not consumed, eliminate the copy.
-///
-/// FIXME: CanonicalizeOSSALifetime replaces this.
 bool SemanticARCOptVisitor::tryPerformOwnedCopyValueOptimization(
     CopyValueInst *cvi) {
-  if (ctx.onlyGuaranteedOpts)
+  // All mandatory copy optimization is handled by CanonicalizeOSSALifetime,
+  // which knows how to preserve lifetimes for debugging.
+  if (ctx.onlyMandatoryOpts)
     return false;
 
   auto originalValue = cvi->getOperand();

lib/SILOptimizer/SemanticARC/LoadCopyToLoadBorrowOpt.cpp

@@ -321,7 +321,7 @@ static bool isWrittenTo(Context &ctx, LoadInst *load,
 bool SemanticARCOptVisitor::visitLoadInst(LoadInst *li) {
   // This optimization can use more complex analysis. We should do some
   // experiments before enabling this by default as a guaranteed optimization.
-  if (ctx.onlyGuaranteedOpts)
+  if (ctx.onlyMandatoryOpts)
     return false;
 
   // If we are not supposed to perform this transform, bail.

lib/SILOptimizer/SemanticARC/OwnershipConversionElimination.cpp

@@ -24,7 +24,7 @@ using namespace semanticarc;
 bool SemanticARCOptVisitor::visitUncheckedOwnershipConversionInst(
     UncheckedOwnershipConversionInst *uoci) {
   // Return false if we are supposed to only be running guaranteed opts.
-  if (ctx.onlyGuaranteedOpts)
+  if (ctx.onlyMandatoryOpts)
     return false;
 
   // Then check if we are running tests and shouldn't perform this optimization

lib/SILOptimizer/SemanticARC/SemanticARCOptVisitor.h

@@ -50,8 +50,8 @@ struct LLVM_LIBRARY_VISIBILITY SemanticARCOptVisitor
   Context ctx;
 
   explicit SemanticARCOptVisitor(SILFunction &fn, DeadEndBlocks &deBlocks,
-                                 bool onlyGuaranteedOpts)
-      : ctx(fn, deBlocks, onlyGuaranteedOpts,
+                                 bool onlyMandatoryOpts)
+      : ctx(fn, deBlocks, onlyMandatoryOpts,
             InstModCallbacks(
                 [this](SILInstruction *inst) { eraseInstruction(inst); },
                 [this](Operand *use, SILValue newValue) {

lib/SILOptimizer/SemanticARC/SemanticARCOpts.cpp

@@ -66,10 +66,10 @@ namespace {
 // case DiagnosticConstantPropagation exposed anything new in this assert
 // configuration.
 struct SemanticARCOpts : SILFunctionTransform {
-  bool guaranteedOptsOnly;
+  bool mandatoryOptsOnly;
 
-  SemanticARCOpts(bool guaranteedOptsOnly)
-      : guaranteedOptsOnly(guaranteedOptsOnly) {}
+  SemanticARCOpts(bool mandatoryOptsOnly)
+      : mandatoryOptsOnly(mandatoryOptsOnly) {}
 
 #ifndef NDEBUG
   void performCommandlineSpecifiedTransforms(SemanticARCOptVisitor &visitor) {
@@ -153,7 +153,7 @@ struct SemanticARCOpts : SILFunctionTransform {
 
     auto *deBlocksAnalysis = getAnalysis<DeadEndBlocksAnalysis>();
     SemanticARCOptVisitor visitor(f, *deBlocksAnalysis->get(&f),
-                                  guaranteedOptsOnly);
+                                  mandatoryOptsOnly);
 
 #ifndef NDEBUG
     // If we are being asked for testing purposes to run a series of transforms
@@ -185,9 +185,9 @@ struct SemanticARCOpts : SILFunctionTransform {
 } // end anonymous namespace
 
 SILTransform *swift::createSemanticARCOpts() {
-  return new SemanticARCOpts(false /*guaranteed*/);
+  return new SemanticARCOpts(false /*mandatory*/);
 }
 
-SILTransform *swift::createGuaranteedARCOpts() {
-  return new SemanticARCOpts(true /*guaranteed*/);
+SILTransform *swift::createMandatoryARCOpts() {
+  return new SemanticARCOpts(true /*mandatory*/);
 }

lib/SILOptimizer/Transforms/CopyPropagation.cpp

@@ -42,9 +42,12 @@ namespace {
 class CopyPropagation : public SILFunctionTransform {
   /// True if debug_value instructions should be pruned.
   bool pruneDebug;
+  /// True of all values should be canonicalized.
+  bool canonicalizeAll;
 
 public:
-  CopyPropagation(bool pruneDebug): pruneDebug(pruneDebug) {}
+  CopyPropagation(bool pruneDebug, bool canonicalizeAll)
+      : pruneDebug(pruneDebug), canonicalizeAll(canonicalizeAll) {}
 
   /// The entry point to this function transformation.
   void run() override;
@@ -83,9 +86,15 @@ void CopyPropagation::run() {
   llvm::SmallSetVector<SILValue, 16> copiedDefs;
   for (auto &bb : *f) {
     for (auto &i : bb) {
-      if (auto *copy = dyn_cast<CopyValueInst>(&i))
+      if (auto *copy = dyn_cast<CopyValueInst>(&i)) {
         copiedDefs.insert(
             CanonicalizeOSSALifetime::getCanonicalCopiedDef(copy));
+      } else if (canonicalizeAll) {
+        if (auto *destroy = dyn_cast<DestroyValueInst>(&i)) {
+          copiedDefs.insert(CanonicalizeOSSALifetime::getCanonicalCopiedDef(
+              destroy->getOperand()));
+        }
+      }
     }
   }
   // Perform copy propgation for each copied value.
@@ -122,14 +131,16 @@ void CopyPropagation::run() {
     accessBlockAnalysis->lockInvalidation();
     invalidateAnalysis(SILAnalysis::InvalidationKind::Instructions);
     accessBlockAnalysis->unlockInvalidation();
-    f->verifyOwnership(deBlocksAnalysis->get(f));
+    if (f->getModule().getOptions().VerifySILOwnership) {
+      f->verifyOwnership(deBlocksAnalysis->get(f));
+    }
   }
 }
 
-SILTransform *swift::createCopyPropagation() {
-  return new CopyPropagation(/*pruneDebug*/ true);
+SILTransform *swift::createMandatoryCopyPropagation() {
+  return new CopyPropagation(/*pruneDebug*/ true, /*canonicalizeAll*/ true);
 }
 
-SILTransform *swift::createMandatoryCopyPropagation() {
-  return new CopyPropagation(/*pruneDebug*/ false);
+SILTransform *swift::createCopyPropagation() {
+  return new CopyPropagation(/*pruneDebug*/ true, /*canonicalizeAll*/ false);
 }

lib/SILOptimizer/Utils/CanonicalOSSALifetime.cpp

@@ -369,7 +369,7 @@ bool CanonicalizeOSSALifetime::computeCanonicalLiveness() {
         continue;
       }
       // Handle debug_value instructions separately.
-      if (pruneDebug) {
+      if (pruneDebugMode) {
         if (auto *dvi = dyn_cast<DebugValueInst>(user)) {
           // Only instructions potentially outside current pruned liveness are
           // interesting.
@@ -647,7 +647,7 @@ void CanonicalizeOSSALifetime::findOrInsertDestroyInBlock(SILBasicBlock *bb) {
   while (true) {
     auto *inst = &*instIter;
 
-    if (pruneDebug) {
+    if (pruneDebugMode) {
       if (auto *dvi = dyn_cast<DebugValueInst>(inst)) {
         if (debugValues.erase(dvi))
           consumes.recordDebugAfterConsume(dvi);
@@ -675,7 +675,8 @@ void CanonicalizeOSSALifetime::findOrInsertDestroyInBlock(SILBasicBlock *bb) {
       return;
     }
     // This is not a potential last user. Keep scanning.
-    // Allow lifetimes to be artificially extended up to the next call.
+    // Allow lifetimes to be artificially extended up to the next call. The goal
+    // is to prevent repeated destroy rewriting without inhibiting optimization.
     if (ApplySite::isa(inst)) {
       existingDestroy = nullptr;
     } else if (!existingDestroy) {
@@ -842,9 +843,10 @@ void CanonicalizeOSSALifetime::rewriteCopies() {
       if (reusedCopyOp) {
         reusedCopyOp->set(srcCopy);
       } else {
-        instsToDelete.insert(srcCopy);
-        LLVM_DEBUG(llvm::dbgs() << "  Removing " << *srcCopy);
-        ++NumCopiesEliminated;
+        if (instsToDelete.insert(srcCopy)) {
+          LLVM_DEBUG(llvm::dbgs() << "  Removing " << *srcCopy);
+          ++NumCopiesEliminated;
+        }
       }
     }
   }

test/IRGen/struct_resilience.swift

@@ -2,7 +2,7 @@
 // RUN: %empty-directory(%t)
 // RUN: %target-swift-frontend -emit-module -enable-library-evolution -emit-module-path=%t/resilient_struct.swiftmodule -module-name=resilient_struct %S/../Inputs/resilient_struct.swift
 // RUN: %target-swift-frontend -emit-module -enable-library-evolution -emit-module-path=%t/resilient_enum.swiftmodule -module-name=resilient_enum -I %t %S/../Inputs/resilient_enum.swift
-// RUN: %target-swift-frontend -module-name struct_resilience -Xllvm -sil-disable-pass=GuaranteedARCOpts -I %t -emit-ir -enable-library-evolution %s | %FileCheck %s
+// RUN: %target-swift-frontend -module-name struct_resilience -Xllvm -sil-disable-pass=MandatoryARCOpts -I %t -emit-ir -enable-library-evolution %s | %FileCheck %s
 // RUN: %target-swift-frontend -module-name struct_resilience -I %t -emit-ir -enable-library-evolution -O %s
 
 import resilient_struct