Merge pull request #84411 from kavon/copyprop-onone

sil: provide ability to run CopyPropagation in -Onone

Author: kavon

include/swift/AST/SILOptions.h

@@ -49,10 +49,14 @@ enum class CopyPropagationOption : uint8_t {
   // just -enable-ossa-modules.
   RequestedPassesOnly,
 
-  // Add all relevant copy propagation passes.  If a setting, e.g.
-  // -enable-ossa-modules, requests to add copy propagation to the pipeline, do
-  // so.
-  On
+  // Run copy propagation during optimized builds only.
+  //
+  // If a setting, e.g. -enable-ossa-modules, requests to add copy propagation
+  // to the performance pipeline, do so.
+  Optimizing,
+
+  // Run copy propagation during all builds and whenever requested.
+  Always
 };
 
 enum class DestroyHoistingOption : uint8_t {
@@ -90,11 +94,8 @@ class SILOptions {
   /// observable end of lexical scope.
   LexicalLifetimesOption LexicalLifetimes = LexicalLifetimesOption::On;
 
-  /// Whether to run SIL copy propagation to shorten object lifetime in whatever
-  /// optimization pipeline is currently used.
-  ///
-  /// When this is 'On' the pipeline has default behavior.
-  CopyPropagationOption CopyPropagation = CopyPropagationOption::On;
+  /// Controls when to run SIL copy propagation, which shortens object lifetimes
+  CopyPropagationOption CopyPropagation = CopyPropagationOption::Optimizing;
 
   /// Whether to run the DestroyAddrHoisting pass.
   ///

include/swift/Option/FrontendOptions.td

@@ -282,12 +282,12 @@ def no_parallel_scan : Flag<["-"], "no-parallel-scan">,
    HelpText<"Perform dependency scanning in a single-threaded fashion.">;
 
 def enable_copy_propagation : Flag<["-"], "enable-copy-propagation">,
-  HelpText<"Run SIL copy propagation with lexical lifetimes to shorten object "
+  HelpText<"Always run SIL copy propagation with lexical lifetimes to shorten object "
            "lifetimes while preserving variable lifetimes.">;
 def copy_propagation_state_EQ :
   Joined<["-"], "enable-copy-propagation=">,
   HelpText<"Whether to enable copy propagation">,
-  MetaVarName<"true|requested-passes-only|false">;
+  MetaVarName<"always|optimizing|requested-passes-only|false">;
 
 def disable_infer_public_concurrent_value : Flag<["-"], "disable-infer-public-sendable">,
     HelpText<"Disable inference of Sendable conformances for public structs and enums">;

lib/DriverTool/sil_opt_main.cpp

@@ -115,8 +115,11 @@ operator<<(raw_ostream &os, const std::optional<CopyPropagationOption> option) {
     case CopyPropagationOption::RequestedPassesOnly:
       os << "requested-passes-only";
       break;
-    case CopyPropagationOption::On:
-      os << "on";
+    case CopyPropagationOption::Optimizing:
+      os << "optimizing";
+      break;
+    case CopyPropagationOption::Always:
+      os << "always";
       break;
     }
   } else {
@@ -135,24 +138,27 @@ class parser<std::optional<CopyPropagationOption>>
   // parse - Return true on error.
   bool parse(Option &O, StringRef ArgName, StringRef Arg,
              std::optional<CopyPropagationOption> &Value) {
-    if (Arg == "" || Arg == "true" || Arg == "TRUE" || Arg == "True" ||
-        Arg == "1") {
-      Value = CopyPropagationOption::On;
+    if (Arg.compare_insensitive("always")) {
+      Value = CopyPropagationOption::Always;
+      return false;
+    }
+    if (Arg.compare_insensitive("optimizing")) {
+      Value = CopyPropagationOption::Optimizing;
       return false;
     }
-    if (Arg == "false" || Arg == "FALSE" || Arg == "False" || Arg == "0") {
+    if (Arg.compare_insensitive("false") || Arg.compare_insensitive("off") ||
+        Arg == "0") {
       Value = CopyPropagationOption::Off;
       return false;
     }
-    if (Arg == "requested-passes-only" || Arg == "REQUESTED-PASSES-ONLY" ||
-        Arg == "Requested-Passes-Only") {
+    if (Arg.compare_insensitive("requested-passes-only")) {
       Value = CopyPropagationOption::RequestedPassesOnly;
       return false;
     }
 
     return O.error("'" + Arg +
-                   "' is invalid for CopyPropagationOption! Try true, false, "
-                   "or requested-passes-only.");
+                   "' is invalid for CopyPropagationOption!"
+                   " Try always, optimizing, or requested-passes-only.");
   }
 
   void initialize() {}
@@ -907,7 +913,7 @@ int sil_opt_main(ArrayRef<const char *> argv, void *MainAddr) {
 
   // Unless overridden below, enabling copy propagation means enabling lexical
   // lifetimes.
-  if (SILOpts.CopyPropagation == CopyPropagationOption::On)
+  if (SILOpts.CopyPropagation >= CopyPropagationOption::Optimizing)
     SILOpts.LexicalLifetimes = LexicalLifetimesOption::On;
 
   // Unless overridden below, disable copy propagation means disabling lexical

lib/Frontend/CompilerInvocation.cpp

@@ -2908,33 +2908,22 @@ static bool ParseSILArgs(SILOptions &Opts, ArgList &Args,
   if (Arg *A = Args.getLastArg(OPT_copy_propagation_state_EQ)) {
     specifiedCopyPropagationOption =
         llvm::StringSwitch<std::optional<CopyPropagationOption>>(A->getValue())
-            .Case("true", CopyPropagationOption::On)
+            .Case("always", CopyPropagationOption::Always)
+            .Case("optimizing", CopyPropagationOption::Optimizing)
             .Case("false", CopyPropagationOption::Off)
             .Case("requested-passes-only",
                   CopyPropagationOption::RequestedPassesOnly)
             .Default(std::nullopt);
+
+    // Error if copy propagation has been set via the flag at the same time.
+    if (auto *Flag = Args.getLastArg(OPT_enable_copy_propagation)) {
+      Diags.diagnose(SourceLoc(), diag::error_invalid_arg_combination,
+                       Flag->getAsString(Args),
+                       A->getAsString(Args));
+    }
   }
   if (Args.hasArg(OPT_enable_copy_propagation)) {
-    if (specifiedCopyPropagationOption) {
-      if (*specifiedCopyPropagationOption == CopyPropagationOption::Off) {
-        // Error if copy propagation has been set to ::Off via the meta-var form
-        // and enabled via the flag.
-        Diags.diagnose(SourceLoc(), diag::error_invalid_arg_combination,
-                       "enable-copy-propagation",
-                       "enable-copy-propagation=false");
-        return true;
-      } else if (*specifiedCopyPropagationOption ==
-                 CopyPropagationOption::RequestedPassesOnly) {
-        // Error if copy propagation has been set to ::RequestedPassesOnly via
-        // the meta-var form and enabled via the flag.
-        Diags.diagnose(SourceLoc(), diag::error_invalid_arg_combination,
-                       "enable-copy-propagation",
-                       "enable-copy-propagation=requested-passes-only");
-        return true;
-      }
-    } else {
-      specifiedCopyPropagationOption = CopyPropagationOption::On;
-    }
+    specifiedCopyPropagationOption = CopyPropagationOption::Always;
   }
   if (specifiedCopyPropagationOption) {
     Opts.CopyPropagation = *specifiedCopyPropagationOption;
@@ -2966,7 +2955,7 @@ static bool ParseSILArgs(SILOptions &Opts, ArgList &Args,
 
   // Unless overridden below, enabling copy propagation means enabling lexical
   // lifetimes.
-  if (Opts.CopyPropagation == CopyPropagationOption::On) {
+  if (Opts.CopyPropagation >= CopyPropagationOption::Optimizing) {
     Opts.LexicalLifetimes = LexicalLifetimesOption::On;
     Opts.DestroyHoisting = DestroyHoistingOption::On;
   }

lib/SILOptimizer/PassManager/PassPipeline.cpp

@@ -255,7 +255,7 @@ static void addMandatoryDiagnosticOptPipeline(SILPassPipelinePlan &P) {
 
   // Only issue weak lifetime warnings for users who select object lifetime
   // optimization. The risk of spurious warnings outweighs the benefits.
-  if (P.getOptions().CopyPropagation == CopyPropagationOption::On) {
+  if (P.getOptions().CopyPropagation >= CopyPropagationOption::Optimizing) {
     P.addDiagnoseLifetimeIssues();
   }
 
@@ -278,6 +278,23 @@ static void addMandatoryDiagnosticOptPipeline(SILPassPipelinePlan &P) {
 
   P.addDiagnoseUnknownConstValues();
   P.addEmbeddedSwiftDiagnostics();
+
+  /// FIXME: Ideally, we'd have this relative order:
+  ///   1. DiagnoseLifetimeIssues
+  ///   2. CopyPropagation
+  ///   3. AddressLowering
+  /// to get the maximum benefits of CopyPropagation + OpaqueValues in -Onone.
+  if (P.getOptions().CopyPropagation == CopyPropagationOption::Always) {
+    // FIXME: ComputeSideEffects helps CopyPropagation simplify across
+    //   call-sites, but PerformanceDiagnostics is sensitive to the # of copies.
+    //   If ManualOwnership is used in the compiler itself, we wouldn't be able
+    //   to bootstrap the compiler on different platforms with same diagnostics.
+#ifdef SWIFT_ENABLE_SWIFT_IN_SWIFT
+    P.addComputeSideEffects();
+#endif
+    P.addMandatoryCopyPropagation();
+  }
+
   P.addPerformanceDiagnostics();
 }
 
@@ -655,7 +672,7 @@ static void addPerfEarlyModulePassPipeline(SILPassPipelinePlan &P) {
   // Cleanup after SILGen: remove trivial copies to temporaries.
   P.addTempRValueElimination();
   // Cleanup after SILGen: remove unneeded borrows/copies.
-  if (P.getOptions().CopyPropagation == CopyPropagationOption::On) {
+  if (P.getOptions().CopyPropagation >= CopyPropagationOption::Optimizing) {
     P.addComputeSideEffects();
     P.addCopyPropagation();
   }

lib/SILOptimizer/SILCombiner/SILCombine.cpp

@@ -668,7 +668,7 @@ class SILCombine : public SILFunctionTransform {
   /// The entry point to the transformation.
   void run() override {
     bool enableCopyPropagation =
-        getOptions().CopyPropagation == CopyPropagationOption::On;
+        getOptions().CopyPropagation >= CopyPropagationOption::Optimizing;
     if (getOptions().EnableOSSAModules) {
       enableCopyPropagation =
           getOptions().CopyPropagation != CopyPropagationOption::Off;

lib/SILOptimizer/Transforms/CopyPropagation.cpp

@@ -688,10 +688,10 @@ void CopyPropagation::verifyOwnership() {
                          : deBlocksAnalysis->get(f));
 }
 
-// MandatoryCopyPropagation is not currently enabled in the -Onone pipeline
-// because it may negatively affect the debugging experience.
+// MandatoryCopyPropagation runs in the -Onone pipeline and needs to be more
+// conservative, preserving debug information.
 SILTransform *swift::createMandatoryCopyPropagation() {
-  return new CopyPropagation(PruneDebugInsts, /*canonicalizeAll*/ true,
+  return new CopyPropagation(DontPruneDebugInsts, /*canonicalizeAll*/ true,
                              /*canonicalizeBorrows*/ false);
 }
 

test/SIL/manual_ownership.swift

@@ -1,4 +1,6 @@
-// RUN: %target-swift-frontend %s -emit-sil -verify -enable-experimental-feature ManualOwnership
+// RUN: %target-swift-frontend %s -emit-sil -verify \
+// RUN:   -enable-experimental-feature ManualOwnership \
+// RUN:   -enable-copy-propagation=always
 
 // REQUIRES: swift_feature_ManualOwnership
 
@@ -12,6 +14,7 @@ public struct Pair {
   var x: Int
   var y: Int
 
+  @_manualOwnership
   consuming func midpoint(_ other: borrowing Pair) -> Pair {
     return Pair(x: (x + other.x) / 2, y: (y + other.y) / 2)
   }
@@ -24,7 +27,10 @@ public class Triangle {
 
   var nontrivial = Whatever()
 
+  @_manualOwnership
   consuming func consuming() {}
+
+  @_manualOwnership
   borrowing func borrowing() {}
 }
 
@@ -33,14 +39,8 @@ public class Triangle {
 @_manualOwnership
 public func basic_return1() -> Triangle {
   let x = Triangle()
-  return x // expected-error {{explicit 'copy' required here}}
+  return x
 }
-@_manualOwnership
-public func basic_return1_fixed() -> Triangle {
-  let x = Triangle()
-  return copy x
-}
-
 
 @_manualOwnership
 public func basic_return2(t: Triangle) -> Triangle {
@@ -56,24 +56,39 @@ public func basic_return3() -> Triangle {
   return Triangle()
 }
 
-// FIXME: we need copy propagation in -Onone to eliminate all these copies
 @_manualOwnership
 func reassign_with_lets() -> Triangle {
   let x = Triangle()
-  let y = x // expected-error {{explicit 'copy' required here}}
-  let z = y // expected-error {{explicit 'copy' required here}}
-  return z // expected-error {{explicit 'copy' required here}}
+  let y = x
+  let z = y
+  return z
 }
 
-// FIXME: we need copy propagation in -Onone to eliminate all but the copies for returning
 @_manualOwnership
 func renamed_return(_ cond: Bool, _ a: Triangle) -> Triangle {
-  let b = a // expected-error {{explicit 'copy' required here}}
-  let c = b // expected-error {{explicit 'copy' required here}}
+  let b = a
+  let c = b
   if cond { return b } // expected-error {{explicit 'copy' required here}}
   return c // expected-error {{explicit 'copy' required here}}
 }
 
+@_manualOwnership
+func renamed_return_fix1(_ cond: Bool, _ a: Triangle) -> Triangle {
+  let b = copy a
+  let c = copy b  // FIXME: not needed! Is explicit_copy_value is blocking propagation?
+  if cond { return b }
+  return c
+}
+
+// FIXME: this crashes CopyPropagation!
+//@_manualOwnership
+//func renamed_return_fix2(_ cond: Bool, _ a: Triangle) -> Triangle {
+//  let b = a
+//  let c = b
+//  if cond { return copy b }
+//  return copy c
+//}
+
 /// MARK: method calls
 
 @_manualOwnership
@@ -87,7 +102,7 @@ func basic_methods_borrowing(_ t1: Triangle) {
 func basic_methods_consuming(_ t1: Triangle) {
   let t2 = Triangle()
   t1.consuming() // expected-error {{explicit 'copy' required here}}
-  t2.consuming() // expected-error {{explicit 'copy' required here}}
+  t2.consuming()
 }
 @_manualOwnership
 func basic_methods_consuming_fixed(_ t1: Triangle) {
@@ -96,7 +111,7 @@ func basic_methods_consuming_fixed(_ t1: Triangle) {
   t3 = Triangle()
 
   (copy t1).consuming()
-  (copy t2).consuming()
+  (copy t2).consuming()  // FIXME: why is this not propagated?
   (copy t3).consuming()
 }
 
@@ -113,12 +128,12 @@ func basic_function_call(_ t1: Triangle) {
 /// MARK: control-flow
 
 
-// FIXME: var's and assignments are a little busted
+// FIXME: var assignments are somtimes impossible to satisfy with 'copy'
 
 // @_manualOwnership
 // func reassignments_1() {
 //   var t3 = Triangle()
-//   t3 = Triangle()
+//   t3 = copy Triangle()  // FIXME: should not be needed
 //   t3.borrowing()
 // }
 // @_manualOwnership