Merge pull request #40539 from nate-chandler/copy_propagation/rename-options

nate-chandler · web-flow · commit 660929dff5b8 · 2021-12-14T07:56:04.000-08:00
[NFC] Tweaked SILOptions fields for copy propagation.
diff --git a/include/swift/AST/SILOptions.h b/include/swift/AST/SILOptions.h
@@ -44,6 +44,20 @@ enum class LexicalLifetimesOption : uint8_t {
   On,
 };
 
+enum class CopyPropagationOption : uint8_t {
+  // Do not add any copy propagation passes.
+  Off = 0,
+
+  // Only add the copy propagation passes requested by other flags, currently
+  // 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
+};
+
 class SILModule;
 
 class SILOptions {
@@ -64,15 +78,12 @@ class SILOptions {
   LexicalLifetimesOption LexicalLifetimes =
       LexicalLifetimesOption::DiagnosticMarkersOnly;
 
-  /// 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
+  /// Whether to 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 DisableCopyPropagation = false;
+  ///
+  /// When this is 'RequestedPassesOnly' the pipeline has default behavior.
+  CopyPropagationOption CopyPropagation =
+      CopyPropagationOption::RequestedPassesOnly;
 
   /// Controls whether the SIL ARC optimizations are run.
   bool EnableARCOptimizations = true;
diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp
@@ -1551,8 +1551,24 @@ static bool ParseSILArgs(SILOptions &Opts, ArgList &Args,
     }
   }
 
-  Opts.EnableCopyPropagation |= Args.hasArg(OPT_enable_copy_propagation);
-  Opts.DisableCopyPropagation |= Args.hasArg(OPT_disable_copy_propagation);
+  if (Args.hasArg(OPT_enable_copy_propagation) &&
+      Args.hasArg(OPT_disable_copy_propagation)) {
+    // Error if copy propagation is enabled and copy propagation is disabled.
+    Diags.diagnose(SourceLoc(), diag::error_invalid_arg_combination,
+                   "enable-copy-propagation", "disable-copy-propagation");
+    return true;
+  } else if (Args.hasArg(OPT_enable_copy_propagation) &&
+             !Args.hasArg(OPT_disable_copy_propagation)) {
+    Opts.CopyPropagation = CopyPropagationOption::On;
+  } else if (!Args.hasArg(OPT_enable_copy_propagation) &&
+             Args.hasArg(OPT_disable_copy_propagation)) {
+    Opts.CopyPropagation = CopyPropagationOption::Off;
+  } else /*if (!Args.hasArg(OPT_enable_copy_propagation) &&
+            !Args.hasArg(OPT_disable_copy_propagation))*/
+  {
+    Opts.CopyPropagation = CopyPropagationOption::RequestedPassesOnly;
+  }
+
   Opts.EnableARCOptimizations &= !Args.hasArg(OPT_disable_arc_opts);
   Opts.EnableOSSAModules |= Args.hasArg(OPT_enable_ossa_modules);
   Opts.EnableOSSAOptimizations &= !Args.hasArg(OPT_disable_ossa_opts);
diff --git a/lib/SILOptimizer/PassManager/PassPipeline.cpp b/lib/SILOptimizer/PassManager/PassPipeline.cpp
@@ -194,10 +194,10 @@ 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().EnableCopyPropagation) {
+  if (P.getOptions().CopyPropagation == CopyPropagationOption::On) {
     P.addDiagnoseLifetimeIssues();
   }
-  
+
   P.addGlobalOpt();
   P.addPerformanceDiagnostics();
   
@@ -412,7 +412,7 @@ 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.
-    if (!P.getOptions().DisableCopyPropagation) {
+    if (P.getOptions().CopyPropagation != CopyPropagationOption::Off) {
       P.addCopyPropagation();
     }
     P.addSemanticARCOpts();
@@ -436,7 +436,7 @@ void addFunctionPasses(SILPassPipelinePlan &P,
 
   // Clean up Semantic ARC before we perform additional post-inliner opts.
   if (P.getOptions().EnableOSSAModules) {
-    if (!P.getOptions().DisableCopyPropagation) {
+    if (P.getOptions().CopyPropagation != CopyPropagationOption::Off) {
       P.addCopyPropagation();
     }
     P.addSemanticARCOpts();
@@ -503,7 +503,7 @@ void addFunctionPasses(SILPassPipelinePlan &P,
 
   // Run a final round of ARC opts when ownership is enabled.
   if (P.getOptions().EnableOSSAModules) {
-    if (!P.getOptions().DisableCopyPropagation) {
+    if (P.getOptions().CopyPropagation != CopyPropagationOption::Off) {
       P.addCopyPropagation();
     }
     P.addSemanticARCOpts();
@@ -539,7 +539,7 @@ static void addPerfEarlyModulePassPipeline(SILPassPipelinePlan &P) {
   // Cleanup after SILGen: remove trivial copies to temporaries.
   P.addTempRValueOpt();
   // Cleanup after SILGen: remove unneeded borrows/copies.
-  if (P.getOptions().EnableCopyPropagation) {
+  if (P.getOptions().CopyPropagation == CopyPropagationOption::On) {
     P.addCopyPropagation();
   }
   P.addSemanticARCOpts();
@@ -863,7 +863,7 @@ SILPassPipelinePlan::getPerformancePassPipeline(const SILOptions &Options) {
   // Run one last copy propagation/semantic arc opts run before serialization/us
   // lowering ownership.
   if (P.getOptions().EnableOSSAModules) {
-    if (!P.getOptions().DisableCopyPropagation) {
+    if (P.getOptions().CopyPropagation != CopyPropagationOption::Off) {
       P.addCopyPropagation();
     }
     P.addSemanticARCOpts();
@@ -915,7 +915,7 @@ SILPassPipelinePlan::getOnonePassPipeline(const SILOptions &Options) {
   P.startPipeline("non-Diagnostic Enabling Mandatory Optimizations");
   P.addForEachLoopUnroll();
   P.addMandatoryCombine();
-  if (P.getOptions().EnableCopyPropagation) {
+  if (P.getOptions().CopyPropagation == CopyPropagationOption::On) {
     // MandatoryCopyPropagation should only be run at -Onone, not -O.
     P.addMandatoryCopyPropagation();
   }
diff --git a/lib/SILOptimizer/SILCombiner/SILCombine.cpp b/lib/SILOptimizer/SILCombiner/SILCombine.cpp
@@ -537,9 +537,11 @@ class SILCombine : public SILFunctionTransform {
     auto *CHA = PM->getAnalysis<ClassHierarchyAnalysis>();
     auto *NLABA = PM->getAnalysis<NonLocalAccessBlockAnalysis>();
 
-    bool enableCopyPropagation = getOptions().EnableCopyPropagation;
+    bool enableCopyPropagation =
+        getOptions().CopyPropagation == CopyPropagationOption::On;
     if (getOptions().EnableOSSAModules) {
-      enableCopyPropagation = !getOptions().DisableCopyPropagation;
+      enableCopyPropagation =
+          getOptions().CopyPropagation != CopyPropagationOption::Off;
     }
 
     SILOptFunctionBuilder FuncBuilder(*this);
diff --git a/tools/sil-opt/SILOpt.cpp b/tools/sil-opt/SILOpt.cpp
@@ -139,11 +139,11 @@ static llvm::cl::opt<bool> EnableOSSAModules(
                    "form when optimizing."));
 
 static llvm::cl::opt<bool>
-    EnableCopyPropagation("enable-copy-propagation",
+    EnableCopyPropagation("enable-copy-propagation", llvm::cl::init(false),
                           llvm::cl::desc("Enable the copy propagation pass."));
 
 static llvm::cl::opt<bool> DisableCopyPropagation(
-    "disable-copy-propagation",
+    "disable-copy-propagation", llvm::cl::init(false),
     llvm::cl::desc("Disable the copy propagation pass."));
 
 namespace {
@@ -524,8 +524,18 @@ int main(int argc, char **argv) {
   SILOpts.EnableSpeculativeDevirtualization = EnableSpeculativeDevirtualization;
   SILOpts.IgnoreAlwaysInline = IgnoreAlwaysInline;
   SILOpts.EnableOSSAModules = EnableOSSAModules;
-  SILOpts.EnableCopyPropagation = EnableCopyPropagation;
-  SILOpts.DisableCopyPropagation = DisableCopyPropagation;
+
+  if (EnableCopyPropagation && DisableCopyPropagation) {
+    fprintf(stderr, "Error! Cannot specify both -enable-copy-propagation "
+                    "and -disable-copy-propagation.");
+    exit(-1);
+  } else if (EnableCopyPropagation && !DisableCopyPropagation) {
+    SILOpts.CopyPropagation = CopyPropagationOption::On;
+  } else if (!EnableCopyPropagation && DisableCopyPropagation) {
+    SILOpts.CopyPropagation = CopyPropagationOption::Off;
+  } else /*if (!EnableCopyPropagation && !DisableCopyPropagation)*/ {
+    SILOpts.CopyPropagation = CopyPropagationOption::RequestedPassesOnly;
+  }
 
   if (EnableCopyPropagation)
     SILOpts.LexicalLifetimes = LexicalLifetimesOption::On;
@@ -538,8 +548,10 @@ int main(int argc, char **argv) {
   bool enableLexicalLifetimes =
       EnableLexicalLifetimes | EnableExperimentalMoveOnly;
   if (enableLexicalLifetimes && !EnableLexicalBorrowScopes) {
-    fprintf(stderr, "Error! Cannot specify both -enable-lexical-lifetimes "
-                    "and either -enable-lexical-borrow-scopes=false");
+    fprintf(
+        stderr,
+        "Error! Cannot specify both -enable-lexical-borrow-scopes=false and "
+        "either -enable-lexical-lifetimes or -enable-experimental-move-only.");
     exit(-1);
   }
   if (enableLexicalLifetimes)
