Disable MandatoryCopyPropagation.

Mandatory copy propagation was primarily a stop-gap until lexcial lifetimes were implemented. It supposedly made variables lifetimes more consistent between -O and -Onone builds. Now that lexical lifetimes are enabled, it is no longer needed for that purpose (and will never satisfactorily meet that goal anyway). Mandatory copy propagation may be enabled again later as a -Onone " optimization. But that requires a more careful audit of the effect on debug information. For now, it should be disabled.
2025-12-14 20:36:38 +01:00 · 2022-02-18 16:47:40 -08:00
parent b3b8fa50c3
commit 4eb7351f4d
10 changed files with 22 additions and 22 deletions
--- a/lib/SILOptimizer/PassManager/PassPipeline.cpp
+++ b/lib/SILOptimizer/PassManager/PassPipeline.cpp
@@ -912,10 +912,7 @@ SILPassPipelinePlan::getOnonePassPipeline(const SILOptions &Options) {
  P.startPipeline("non-Diagnostic Enabling Mandatory Optimizations");
  P.addForEachLoopUnroll();
  P.addMandatoryCombine();
-  if (P.getOptions().CopyPropagation == CopyPropagationOption::On) {
-    // 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();
--- a/lib/SILOptimizer/Transforms/CopyPropagation.cpp
+++ b/lib/SILOptimizer/Transforms/CopyPropagation.cpp
@@ -570,6 +570,8 @@ void CopyPropagation::run() {
  }
 }

+// MandatoryCopyPropagation is not currently enabled in the -Onone pipeline
+// because it may negatively affect the debugging experience.
 SILTransform *swift::createMandatoryCopyPropagation() {
  return new CopyPropagation(/*pruneDebug*/ true, /*canonicalizeAll*/ true,
                             /*canonicalizeBorrows*/ false,
@@ -581,4 +583,3 @@ SILTransform *swift::createCopyPropagation() {
                             /*canonicalizeBorrows*/ EnableRewriteBorrows,
                             /*poisonRefs*/ false);
 }
-
--- a/test/DebugInfo/if-branchlocations.swift
+++ b/test/DebugInfo/if-branchlocations.swift
@@ -1,5 +1,4 @@
-// RUN: %target-swift-frontend %s -emit-sil -enable-copy-propagation=false -emit-verbose-sil -g -o - | %FileCheck %s --check-prefixes=CHECK,CHECK-NCP
-// RUN: %target-swift-frontend %s -emit-sil -enable-copy-propagation -enable-lexical-borrow-scopes=false -emit-verbose-sil -g -o - | %FileCheck %s --check-prefixes=CHECK,CHECK-CP
+// RUN: %target-swift-frontend %s -emit-sil -emit-verbose-sil -g -o - | %FileCheck %s --check-prefixes=CHECK

 class NSURL {}

@@ -27,8 +26,6 @@ class AppDelegate {
        // Verify that the branch's location is >= the cleanup's location.
        // (The implicit false block of the conditional
        //  below inherits the location from the condition.)
-        // CHECK-CP: strong_release{{.*}}$NSPathControlItem{{.*}}line:[[@LINE+3]]
-        // CHECK-CP: debug_value [poison] {{.*}}$NSPathControlItem, let, name "item"{{.*}}line:[[@LINE-7]]:14:in_prologue
        // CHECK: br{{.*}}line:[[@LINE+1]]
        if let url = item.URL
        {
--- a/test/DebugInfo/linetable-do.swift
+++ b/test/DebugInfo/linetable-do.swift
@@ -1,6 +1,5 @@
 // RUN: %target-swift-frontend -Xllvm -sil-full-demangle %s -emit-ir -g -o - | %FileCheck %s
-// RUN: %target-swift-frontend -Xllvm -sil-full-demangle -enable-copy-propagation=false %s -emit-sil -emit-verbose-sil -g -o - | %FileCheck --check-prefixes=CHECK-SIL,CHECK-NCP %s
-// RUN: %target-swift-frontend -Xllvm -sil-full-demangle -enable-copy-propagation -enable-lexical-borrow-scopes=false %s -emit-sil -emit-verbose-sil -g -o - | %FileCheck --check-prefixes=CHECK-SIL,CHECK-CP %s
+// RUN: %target-swift-frontend -Xllvm -sil-full-demangle %s -emit-sil -emit-verbose-sil -g -o - | %FileCheck --check-prefixes=CHECK-SIL %s
 import StdlibUnittest

 class Obj {}
@@ -18,10 +17,9 @@ func testDoStmt() throws -> Void {
    let obj = Obj()
    _blackHole(obj)
    // The poison debug_value takes the location of the original decl.
-    // CHECK-CP:  debug_value [poison] %{{.*}} : $Obj{{.*}} line:[[@LINE-3]]:9:in_prologue
    try foo(100)
    // CHECK-SIL: bb{{.*}}(%{{[0-9]+}} : $()):
-    // CHECK-NCP-NEXT: strong_release {{.*}}: $Obj{{.*}} line:[[@LINE+1]]:3:cleanup
+    // CHECK-SIL-NEXT: strong_release {{.*}}: $Obj{{.*}} line:[[@LINE+1]]:3:cleanup
  }
  // CHECK-SIL-NEXT:     = tuple ()
  // CHECK-SIL-NEXT:   return                        {{.*}} line:[[@LINE+1]]
--- a/test/Distributed/SIL/distributed_actor_default_init_sil_5.swift
+++ b/test/Distributed/SIL/distributed_actor_default_init_sil_5.swift
@@ -64,6 +64,8 @@ distributed actor MyDistActor {

 // CHECK: [[CONTINUE]]:
 // CHECK:        hop_to_executor [[SELF]] : $MyDistActor
+// One of the following retain_value operations could be optimized away.
+// CHECK-NEXT:   retain_value [[SYSTEM]] : $FakeActorSystem
 // CHECK-NEXT:   retain_value [[SYSTEM]] : $FakeActorSystem
 // CHECK-NEXT:   // function_ref FakeActorSystem.actorReady<A>(_:)
 // CHECK-NEXT:   [[READY_FN:%[0-9]+]] = function_ref @$s27FakeDistributedActorSystems0aC6SystemV10actorReadyyyx01_B00bC0RzAA0C7AddressV2IDRtzlF : $@convention(method) <τ_0_0 where τ_0_0 : DistributedActor, τ_0_0.ID == ActorAddress> (@guaranteed τ_0_0, @guaranteed FakeActorSystem) -> ()
--- a/test/IRGen/debug_poison.swift
+++ b/test/IRGen/debug_poison.swift
@@ -1,4 +1,11 @@
 // RUN: %target-swift-frontend -primary-file %s -emit-ir -Onone -enable-copy-propagation -enable-lexical-borrow-scopes=false | %FileCheck %s -DINT=i%target-ptrsize
+//
+// This test is currently disabled because mandatory copy propagation
+// is not part of the pipeline. It may be re-added to the pipeline,
+// but it isn't clear if we'll still need to emit poison references by
+// that time.
+//
+// REQUIRES: mandatory_copy_propagation

 // Test debug_value [poison] emission

--- a/test/IRGen/unmanaged_objc_throw_func.swift
+++ b/test/IRGen/unmanaged_objc_throw_func.swift
@@ -36,16 +36,12 @@ import Foundation

 // CHECK: [[L2]]:                                     ; preds = %entry
 // CHECK-NEXT: %[[T4:.+]] = phi %TSo10CFArrayRefa* [ %[[T0]], %entry ]
-// CHECK-NEXT: %[[T4a:.+]] = bitcast %T25unmanaged_objc_throw_func9SR_9035_CC* %{{.+}} to i8*
-// CHECK-NEXT: call void @llvm.objc.release(i8* %[[T4a]])
 // CHECK-NEXT: %[[T5:.+]] = ptrtoint %TSo10CFArrayRefa* %[[T4]] to i{{32|64}}
 // CHECK-NEXT: br label %[[L3:.+]]

 // CHECK: [[L1]]:                                     ; preds = %entry
 // CHECK-NEXT: %[[T6:.+]] = phi %swift.error* [ %[[T2]], %entry ]
 // CHECK-NEXT: store %swift.error* null, %swift.error** %swifterror, align {{[0-9]+}}
-// CHECK-NEXT: %[[T6a:.+]] = bitcast %T25unmanaged_objc_throw_func9SR_9035_CC* %{{.+}} to i8*
-// CHECK-NEXT: call void @llvm.objc.release(i8* %[[T6a]])
 // CHECK-NEXT: %[[T7:.+]] = icmp eq i{{32|64}} %{{.+}}, 0
 // CHECK-NEXT: br i1 %[[T7]], label %[[L4:.+]], label %[[L5:.+]]

@@ -70,5 +66,7 @@ import Foundation

 // CHECK: [[L3]]:                                     ; preds = %[[L2]], %[[L7]]
 // CHECK-NEXT: %[[T12:.+]] = phi i{{32|64}} [ 0, %[[L7]] ], [ %[[T5]], %[[L2]] ]
+// CHECK-NEXT: %[[T13:.+]] = bitcast %T25unmanaged_objc_throw_func9SR_9035_CC* %{{.+}} to i8*
+// CHECK-NEXT: call void @llvm.objc.release(i8* %[[T13]])
 // CHECK-NEXT: %[[T14:.+]] = inttoptr i{{32|64}} %[[T12]] to %struct.__CFArray*
 // CHECK-NEXT: ret %struct.__CFArray* %[[T14]]
--- a/test/Interpreter/builtin_bridge_object.swift
+++ b/test/Interpreter/builtin_bridge_object.swift
@@ -65,7 +65,6 @@ if true {
  // CHECK-NEXT: true
  print(x === x2)
  // CHECK-OPT-NEXT: deallocated
-  // CHECK-DBG-NEXT: deallocated

  print(nonPointerBits(bo) == 0)
  // CHECK-NEXT: true
@@ -79,6 +78,7 @@ if true {
  _fixLifetime(bo3)
  _fixLifetime(bo4)
 }
+// CHECK-DBG-NEXT: deallocated
 // CHECK-NEXT: deallocated

 // Try with all spare bits set.
@@ -94,7 +94,6 @@ if true {
  // CHECK-NEXT: true
  print(x === x2)
  // CHECK-OPT-NEXT: deallocated
-  // CHECK-DBG-NEXT: deallocated
  
  print(nonPointerBits(bo) == NATIVE_SPARE_BITS)
  // CHECK-NEXT: true
@@ -108,6 +107,7 @@ if true {
  _fixLifetime(bo3)
  _fixLifetime(bo4)
 }
+// CHECK-DBG-NEXT: deallocated
 // CHECK-NEXT: deallocated


--- a/test/SILGen/moveonly_builtin.swift
+++ b/test/SILGen/moveonly_builtin.swift
@@ -31,10 +31,10 @@ class Klass {}
 // CHECK-SIL-NEXT: debug_value
 // CHECK-SIL-NEXT: strong_retain
 // CHECK-SIL-NEXT: move_value
-// CHECK-SIL-NEXT: strong_release
 // CHECK-SIL-NEXT: debug_value [moved] undef
 // CHECK-SIL-NEXT: tuple
 // CHECK-SIL-NEXT: tuple
+// CHECK-SIL-NEXT: strong_release
 // CHECK-SIL-NEXT: return
 // CHECK-SIL: } // end sil function '$s8moveonly7useMoveyAA5KlassCADnF'
 func useMove(_ k: __owned Klass) -> Klass {
@@ -65,10 +65,10 @@ func useMove(_ k: __owned Klass) -> Klass {
 // CHECK-SIL-NEXT: debug_value
 // CHECK-SIL-NEXT: strong_retain
 // CHECK-SIL-NEXT: move_value
-// CHECK-SIL-NEXT: strong_release
 // CHECK-SIL-NEXT: debug_value [moved] undef
 // CHECK-SIL-NEXT: tuple
 // CHECK-SIL-NEXT: tuple
+// CHECK-SIL-NEXT: strong_release
 // CHECK-SIL-NEXT: return
 // CHECK-SIL: } // end sil function '$s8moveonly7useMoveyxxnRlzClF'
 func useMove<T : AnyObject>(_ k: __owned T) -> T {
--- a/test/sil-passpipeline-dump/basic.test-sh
+++ b/test/sil-passpipeline-dump/basic.test-sh
@@ -2,7 +2,7 @@

 // CHECK: ---
 // CHECK: name:            non-Diagnostic Enabling Mandatory Optimizations
-// CHECK: passes:          [ "for-each-loop-unroll", "mandatory-combine", "mandatory-copy-propagation",
+// CHECK: passes:          [ "for-each-loop-unroll", "mandatory-combine",
 // CHECK: "mandatory-arc-opts" ]
 // CHECK: ---
 // CHECK: name:            Serialization