From 4a943d464d5bb06650d492b57f9fa73ee9e7180a Mon Sep 17 00:00:00 2001 From: Kavon Farvardin Date: Fri, 19 Sep 2025 15:12:50 -0700 Subject: [PATCH] sil: provide ability to run CopyPropagation in -Onone This does not enable it by default. Use either of the flags: ``` -enable-copy-propagation -enable-copy-propagation=always ``` to enable it in -Onone. The previous frontend flag `-enable-copy-propagation=true` has been renamed to `-enable-copy-propagation=optimizing`, which is currently default. rdar://107610971 --- include/swift/AST/SILOptions.h | 19 +++-- include/swift/Option/FrontendOptions.td | 4 +- lib/DriverTool/sil_opt_main.cpp | 28 ++++--- lib/Frontend/CompilerInvocation.cpp | 33 +++----- lib/SILOptimizer/PassManager/PassPipeline.cpp | 21 ++++- lib/SILOptimizer/SILCombiner/SILCombine.cpp | 2 +- .../Transforms/CopyPropagation.cpp | 6 +- test/SIL/manual_ownership.swift | 53 +++++++----- test/SILOptimizer/copy_propagation.swift | 84 ++++++++++++++++++- ...sable-copy-propagation-frontend-flag.swift | 6 +- test/SILOptimizer/shrink_borrow_scope.swift | 2 +- 11 files changed, 184 insertions(+), 74 deletions(-) diff --git a/include/swift/AST/SILOptions.h b/include/swift/AST/SILOptions.h index fd4385e238a3e..f7391c5916dd5 100644 --- a/include/swift/AST/SILOptions.h +++ b/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. /// diff --git a/include/swift/Option/FrontendOptions.td b/include/swift/Option/FrontendOptions.td index c10e15a246409..60d5c13a7bd13 100644 --- a/include/swift/Option/FrontendOptions.td +++ b/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">; diff --git a/lib/DriverTool/sil_opt_main.cpp b/lib/DriverTool/sil_opt_main.cpp index 8e74ad4a7d15c..682b3150b22ac 100644 --- a/lib/DriverTool/sil_opt_main.cpp +++ b/lib/DriverTool/sil_opt_main.cpp @@ -115,8 +115,11 @@ operator<<(raw_ostream &os, const std::optional 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> // parse - Return true on error. bool parse(Option &O, StringRef ArgName, StringRef Arg, std::optional &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() {} @@ -908,7 +914,7 @@ int sil_opt_main(ArrayRef 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 diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp index 0eba0324b33ac..91e9a565527f7 100644 --- a/lib/Frontend/CompilerInvocation.cpp +++ b/lib/Frontend/CompilerInvocation.cpp @@ -2878,33 +2878,22 @@ static bool ParseSILArgs(SILOptions &Opts, ArgList &Args, if (Arg *A = Args.getLastArg(OPT_copy_propagation_state_EQ)) { specifiedCopyPropagationOption = llvm::StringSwitch>(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; @@ -2936,7 +2925,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; } diff --git a/lib/SILOptimizer/PassManager/PassPipeline.cpp b/lib/SILOptimizer/PassManager/PassPipeline.cpp index 70dbb533e58f1..e3787c74967bb 100644 --- a/lib/SILOptimizer/PassManager/PassPipeline.cpp +++ b/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(); } diff --git a/lib/SILOptimizer/SILCombiner/SILCombine.cpp b/lib/SILOptimizer/SILCombiner/SILCombine.cpp index 6ff18e4012d79..1c6d9ba79cfc9 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombine.cpp +++ b/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; diff --git a/lib/SILOptimizer/Transforms/CopyPropagation.cpp b/lib/SILOptimizer/Transforms/CopyPropagation.cpp index 894a8251dd487..67c606995e1fc 100644 --- a/lib/SILOptimizer/Transforms/CopyPropagation.cpp +++ b/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); } diff --git a/test/SIL/manual_ownership.swift b/test/SIL/manual_ownership.swift index 14bd0ac0542fe..a5290fc0c5615 100644 --- a/test/SIL/manual_ownership.swift +++ b/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 diff --git a/test/SILOptimizer/copy_propagation.swift b/test/SILOptimizer/copy_propagation.swift index 66f07e127472d..edb06285af2c3 100644 --- a/test/SILOptimizer/copy_propagation.swift +++ b/test/SILOptimizer/copy_propagation.swift @@ -1,4 +1,8 @@ -// RUN: %target-swift-frontend -primary-file %s -O -sil-verify-all -module-name=test -emit-sil | %FileCheck %s +// RUN: %target-swift-frontend -primary-file %s -O \ +// RUN: -sil-verify-all -module-name=test -emit-sil | %FileCheck %s + +// RUN: %target-swift-frontend -primary-file %s -Onone -enable-copy-propagation \ +// RUN: -sil-verify-all -module-name=test -emit-sil | %FileCheck %s --check-prefix ONONE // REQUIRES: swift_in_compiler @@ -21,6 +25,20 @@ func borrow(_ c: C) // CHECK: [[NON_BARRIER:%[^,]+]] = function_ref @non_barrier // CHECK: apply [[NON_BARRIER]]() // CHECK-LABEL: } // end sil function 'test_hoist_over_non_barrier' + + +///// +// NOTE: in -Onone, the release is NOT hoisted, to preserve debugging. + +// ONONE-LABEL: sil {{.*}}@test_hoist_over_non_barrier : {{.*}} { +// ONONE: [[INSTANCE:%[^,]+]] = apply {{.*}} -> @owned C +// ONONE: debug_value [[INSTANCE]], let, name "c" +// ONONE: [[BORROW:%[^,]+]] = function_ref @borrow +// ONONE: apply [[BORROW]]([[INSTANCE]]) +// ONONE: [[NON_BARRIER:%[^,]+]] = function_ref @non_barrier +// ONONE: apply [[NON_BARRIER]]() +// ONONE: strong_release [[INSTANCE]] +// ONONE-LABEL: } // end sil function 'test_hoist_over_non_barrier' @_silgen_name("test_hoist_over_non_barrier") func test_hoist_over_non_barrier() { let c = C() @@ -28,3 +46,67 @@ func test_hoist_over_non_barrier() { non_barrier() } +// CHECK-LABEL: sil {{.*}}@reassign_with_lets : {{.*}} { +// CHECK: [[INSTANCE:%[^,]+]] = alloc_ref $C +// CHECK: [[EI:%.*]] = end_init_let_ref [[INSTANCE]] +// CHECK-NEXT: debug_value [[EI]], let, name "x" +// CHECK-NEXT: debug_value [[EI]], let, name "y" +// CHECK-NEXT: debug_value [[EI]], let, name "z" +// CHECK-NEXT: return [[EI]] +// CHECK-LABEL: } // end sil function 'reassign_with_lets' + +// ONONE-LABEL: sil {{.*}}@reassign_with_lets : {{.*}} { +// ONONE: [[INSTANCE:%[^,]+]] = apply {{.*}} -> @owned C +// ONONE-NEXT: debug_value [[INSTANCE]], let, name "x" +// ONONE-NEXT: debug_value [[INSTANCE]], let, name "y" +// ONONE-NEXT: debug_value [[INSTANCE]], let, name "z" +// ONONE-NEXT: return [[INSTANCE]] +// ONONE-LABEL: } // end sil function 'reassign_with_lets' +@_silgen_name("reassign_with_lets") +func reassign_with_lets() -> C { + let x = C() + let y = x + let z = y + return z +} + +// CHECK-LABEL: sil {{.*}}@renamed_return : {{.*}} { +// CHECK-NOT: strong_retain +// CHECK: debug_value %1, let, name "a" +// CHECK-NEXT: debug_value %1, let, name "b" +// CHECK-NEXT: debug_value %1, let, name "c" +// CHECK-NEXT: strong_retain %1 +// CHECK-NEXT: return %1 +// CHECK-LABEL: } // end sil function 'renamed_return' + + +///// +// Slightly different control-flow in -Onone, but still one +// retain dynamically executed on the object. + +// ONONE-LABEL: sil {{.*}}@renamed_return : {{.*}} { +// ONONE-NOT: strong_retain +// ONONE: debug_value %1, let, name "a" +// ONONE-NEXT: debug_value %1, let, name "b" +// ONONE-NEXT: debug_value %1, let, name "c" +// ONONE: cond_br {{.*}} bb1, bb2 + +// ONONE: bb1 +// ONONE: strong_retain %1 +// ONONE: br bb3 + +// ONONE: bb2 +// ONONE: strong_retain %1 +// ONONE: br bb3 + +// ONONE: bb3 +// ONONE-NEXT: return +// ONONE-LABEL: } // end sil function 'renamed_return' +@_silgen_name("renamed_return") +func renamed_return(_ cond: Bool, _ a: C) -> C { + let b = a + let c = b + if cond { return b } + return c +} + diff --git a/test/SILOptimizer/disable-copy-propagation-frontend-flag.swift b/test/SILOptimizer/disable-copy-propagation-frontend-flag.swift index 91ed913dd4f91..194db5aa3c26f 100644 --- a/test/SILOptimizer/disable-copy-propagation-frontend-flag.swift +++ b/test/SILOptimizer/disable-copy-propagation-frontend-flag.swift @@ -49,7 +49,7 @@ // RUN: %target-swift-frontend %s \ // RUN: -O \ -// RUN: -enable-copy-propagation=true \ +// RUN: -enable-copy-propagation=optimizing \ // RUN: -Xllvm -sil-print-pass-name \ // RUN: -emit-ir \ // RUN: -o /dev/null \ @@ -57,7 +57,7 @@ // RUN: %target-swift-frontend %s \ // RUN: -O \ -// RUN: -enable-copy-propagation=true \ +// RUN: -enable-copy-propagation=optimizing \ // RUN: -enable-destroy-hoisting=false \ // RUN: -Xllvm -sil-print-pass-name \ // RUN: -emit-ir \ @@ -66,7 +66,7 @@ // RUN: %target-swift-frontend %s \ // RUN: -O \ -// RUN: -enable-copy-propagation=true \ +// RUN: -enable-copy-propagation=optimizing \ // RUN: -enable-destroy-hoisting=true \ // RUN: -Xllvm -sil-print-pass-name \ // RUN: -emit-ir \ diff --git a/test/SILOptimizer/shrink_borrow_scope.swift b/test/SILOptimizer/shrink_borrow_scope.swift index 6cfb9aeb935e1..f7d28621e66b0 100644 --- a/test/SILOptimizer/shrink_borrow_scope.swift +++ b/test/SILOptimizer/shrink_borrow_scope.swift @@ -1,4 +1,4 @@ -// RUN: %target-swift-frontend -c -O -enable-copy-propagation=true -enable-lexical-lifetimes=true -sil-verify-all -Xllvm -sil-print-final-ossa-module %s | %FileCheck %s +// RUN: %target-swift-frontend -c -O -enable-copy-propagation=optimizing -enable-lexical-lifetimes=true -sil-verify-all -Xllvm -sil-print-final-ossa-module %s | %FileCheck %s // ============================================================================= // = DECLARATIONS {{