Merge pull request #78384 from eeckstein/enable-code-motion-in-ossa

Optimizer: enable SILCodeMotion in OSSA, but only for trivial values.

Author: eeckstein

SwiftCompilerSources/Sources/Optimizer/Utilities/PhiUpdater.swift

@@ -126,7 +126,7 @@ private func createBorrowedFrom(for phi: Phi, _ context: some MutatingContext) {
   if !phi.value.uses.contains(where: { $0.forwardingBorrowedFromUser != nil }) {
     let builder = Builder(atBeginOf: phi.value.parentBlock, context)
     let bfi = builder.createBorrowedFrom(borrowedValue: phi.value, enclosingValues: [])
-    phi.value.uses.ignore(user: bfi).replaceAll(with: bfi, context)
+    phi.value.uses.ignoreUsers(ofType: BorrowedFromInst.self).replaceAll(with: bfi, context)
   }
 }
 

include/swift/SILOptimizer/Utils/OwnershipOptUtils.h

@@ -367,6 +367,8 @@ void updateGuaranteedPhis(SILPassManager *pm, ArrayRef<SILPhiArgument *> phis);
 /// Replaces phis with the unique incoming values if all incoming values are the same.
 void replacePhisWithIncomingValues(SILPassManager *pm, ArrayRef<SILPhiArgument *> phis);
 
+bool hasOwnershipOperandsOrResults(SILInstruction *inst);
+
 } // namespace swift
 
 #endif

lib/SILOptimizer/LoopTransforms/LICM.cpp

@@ -32,6 +32,7 @@
 #include "swift/SILOptimizer/PassManager/Transforms.h"
 #include "swift/SILOptimizer/Utils/CFGOptUtils.h"
 #include "swift/SILOptimizer/Utils/InstOptUtils.h"
+#include "swift/SILOptimizer/Utils/OwnershipOptUtils.h"
 #include "swift/SILOptimizer/Utils/SILSSAUpdater.h"
 
 #include "llvm/ADT/DepthFirstIterator.h"
@@ -849,21 +850,6 @@ static bool analyzeBeginAccess(BeginAccessInst *BI,
   return true;
 }
 
-static bool hasOwnershipOperandsOrResults(SILInstruction *inst) {
-  if (!inst->getFunction()->hasOwnership())
-    return false;
-
-  for (SILValue result : inst->getResults()) {
-    if (result->getOwnershipKind() != OwnershipKind::None)
-      return true;
-  }
-  for (Operand &op : inst->getAllOperands()) {
-    if (op.get()->getOwnershipKind() != OwnershipKind::None)
-      return true;
-  }
-  return false;
-}
-
 // Analyzes current loop for hosting/sinking potential:
 // Computes set of instructions we may be able to move out of the loop
 // Important Note:

lib/SILOptimizer/Transforms/SILCodeMotion.cpp

@@ -27,6 +27,7 @@
 #include "swift/SILOptimizer/PassManager/Passes.h"
 #include "swift/SILOptimizer/PassManager/Transforms.h"
 #include "swift/SILOptimizer/Utils/InstOptUtils.h"
+#include "swift/SILOptimizer/Utils/OwnershipOptUtils.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Support/CommandLine.h"
@@ -917,6 +918,8 @@ static const int SinkSearchWindow = 6;
 
 /// Returns True if we can sink this instruction to another basic block.
 static bool canSinkInstruction(SILInstruction *Inst) {
+  if (hasOwnershipOperandsOrResults(Inst))
+    return false;
   return !Inst->hasUsesOfAnyResult() && !isa<TermInst>(Inst);
 }
 
@@ -1178,6 +1181,9 @@ static bool sinkArgument(EnumCaseDataflowContext &Context, SILBasicBlock *BB, un
   if (!FSI || !hasOneNonDebugUse(FSI))
     return false;
 
+  if (hasOwnershipOperandsOrResults(FSI))
+    return false;
+
   // The list of identical instructions.
   SmallVector<SingleValueInstruction *, 8> Clones;
   Clones.push_back(FSI);
@@ -1743,38 +1749,43 @@ static bool processFunction(SILFunction *F, AliasAnalysis *AA,
     LLVM_DEBUG(llvm::dbgs() << "    Merging predecessors!\n");
     State.mergePredecessorStates();
 
-    // If our predecessors cover any of our enum values, attempt to hoist
-    // releases up the CFG onto enum payloads or sink retains out of switch
-    // regions.
-    LLVM_DEBUG(llvm::dbgs() << "    Attempting to move releases into "
-                               "predecessors!\n");
-
-    // Perform a relatively local forms of retain sinking and release hoisting
-    // regarding switch regions and SILargument. This are not handled by retain
-    // release code motion.
-    if (HoistReleases) {
-      Changed |= State.hoistDecrementsIntoSwitchRegions(AA);
-    }
+    if (!F->hasOwnership()) {
+      // If our predecessors cover any of our enum values, attempt to hoist
+      // releases up the CFG onto enum payloads or sink retains out of switch
+      // regions.
+      LLVM_DEBUG(llvm::dbgs() << "    Attempting to move releases into "
+                                 "predecessors!\n");
+
+      // Perform a relatively local forms of retain sinking and release hoisting
+      // regarding switch regions and SILargument. This are not handled by retain
+      // release code motion.
+      if (HoistReleases) {
+        Changed |= State.hoistDecrementsIntoSwitchRegions(AA);
+      }
 
-    // Sink switch related retains.
-    Changed |= sinkIncrementsIntoSwitchRegions(State.getBB(), AA, RCIA);
-    Changed |= State.sinkIncrementsOutOfSwitchRegions(AA, RCIA);
+      // Sink switch related retains.
+      Changed |= sinkIncrementsIntoSwitchRegions(State.getBB(), AA, RCIA);
+      Changed |= State.sinkIncrementsOutOfSwitchRegions(AA, RCIA);
 
-    // Then attempt to sink code from predecessors. This can include retains
-    // which is why we always attempt to move releases up the CFG before sinking
-    // code from predecessors. We will never sink the hoisted releases from
-    // predecessors since the hoisted releases will be on the enum payload
-    // instead of the enum itself.
-    Changed |= canonicalizeRefCountInstrs(State.getBB());
+      // Then attempt to sink code from predecessors. This can include retains
+      // which is why we always attempt to move releases up the CFG before sinking
+      // code from predecessors. We will never sink the hoisted releases from
+      // predecessors since the hoisted releases will be on the enum payload
+      // instead of the enum itself.
+      Changed |= canonicalizeRefCountInstrs(State.getBB());
+    }
     Changed |= sinkCodeFromPredecessors(BBToStateMap, State.getBB());
     Changed |= sinkArgumentsFromPredecessors(BBToStateMap, State.getBB());
     Changed |= sinkLiteralsFromPredecessors(State.getBB());
-    // Try to hoist release of a SILArgument to predecessors.
-    Changed |= hoistSILArgumentReleaseInst(State.getBB());
 
-    // Then perform the dataflow.
-    LLVM_DEBUG(llvm::dbgs() << "    Performing the dataflow!\n");
-    Changed |= State.process();
+    if (!F->hasOwnership()) {
+      // Try to hoist release of a SILArgument to predecessors.
+      Changed |= hoistSILArgumentReleaseInst(State.getBB());
+
+      // Then perform the dataflow.
+      LLVM_DEBUG(llvm::dbgs() << "    Performing the dataflow!\n");
+      Changed |= State.process();
+    }
   }
 
   return Changed;
@@ -1791,9 +1802,6 @@ class SILCodeMotion : public SILFunctionTransform {
   /// The entry point to the transformation.
   void run() override {
     auto *F = getFunction();
-    // Skip functions with ownership for now.
-    if (F->hasOwnership())
-      return;
 
     auto *AA = getAnalysis<AliasAnalysis>(F);
     auto *PO = getAnalysis<PostOrderAnalysis>()->get(F);

lib/SILOptimizer/Utils/OwnershipOptUtils.cpp

@@ -1951,3 +1951,18 @@ void swift::replacePhisWithIncomingValues(SILPassManager *pm, ArrayRef<SILPhiArg
   }
   replacePhisWithIncomingValuesFunction({pm->getSwiftPassInvocation()}, ArrayRef(bridgedPhis));
 }
+
+bool swift::hasOwnershipOperandsOrResults(SILInstruction *inst) {
+  if (!inst->getFunction()->hasOwnership())
+    return false;
+
+  for (SILValue result : inst->getResults()) {
+    if (result->getOwnershipKind() != OwnershipKind::None)
+      return true;
+  }
+  for (Operand &op : inst->getAllOperands()) {
+    if (op.get()->getOwnershipKind() != OwnershipKind::None)
+      return true;
+  }
+  return false;
+}

test/SILOptimizer/earlycodemotion.sil

@@ -522,6 +522,36 @@ bb3(%12 : $Bool):
   return %12 : $Bool
 }
 
+// CHECK-LABEL: sil [ossa] @sink_struct_ossa :
+// CHECK:       bb0({{.*}}):
+// CHECK:         switch_enum
+// CHECK:       bb1({{.*}}):
+// CHECK:         integer_literal $Builtin.Int1, -1
+// CHECK:         br bb3({{.*}} : $Builtin.Int1)
+// CHECK:       bb2:
+// CHECK:         integer_literal $Builtin.Int1, 0
+// CHECK:         br bb3({{.*}} : $Builtin.Int1)
+// CHECK:       bb3({{.*}} : $Builtin.Int1):
+// CHECK:         struct $Bool
+// CHECK:       } // end sil function 'sink_struct_ossa'
+sil [ossa] @sink_struct_ossa : $@convention(thin) (Optional<Builtin.Int32>) -> Bool {
+bb0(%0 : $Optional<Builtin.Int32>):
+  switch_enum %0 : $Optional<Builtin.Int32>, case #Optional.some!enumelt: bb1, case #Optional.none!enumelt: bb2
+
+bb1(%2 : $Builtin.Int32):
+  %6 = integer_literal $Builtin.Int1, -1
+  %7 = struct $Bool (%6 : $Builtin.Int1)
+  br bb3(%7 : $Bool)
+
+bb2:
+  %9 = integer_literal $Builtin.Int1, 0
+  %10 = struct $Bool (%9 : $Builtin.Int1)
+  br bb3(%10 : $Bool)
+
+bb3(%12 : $Bool):
+  return %12 : $Bool
+}
+
 // Sink retain down the successors so we can pair up retain with release on the
 // fast path.
 class Test {
@@ -1402,6 +1432,26 @@ bb3(%z1 : $Builtin.RawPointer):
   return %z1 : $Builtin.RawPointer
 }
 
+// CHECK-LABEL: sil [ossa] @sink_literals_through_arguments_ossa :
+// CHECK:         string_literal utf8 "0"
+// CHECK-NEXT:    return
+// CHECK:       } // end sil function 'sink_literals_through_arguments_ossa'
+sil [ossa] @sink_literals_through_arguments_ossa : $@convention(thin) (Builtin.Int1) -> Builtin.RawPointer {
+bb0(%0 : $Builtin.Int1):
+  cond_br %0, bb1, bb2
+
+bb1:
+  %x1 = string_literal utf8 "0"
+  br bb3(%x1 : $Builtin.RawPointer)
+
+bb2:
+  %y1 = string_literal utf8 "0"
+  br bb3(%y1 : $Builtin.RawPointer)
+
+bb3(%z1 : $Builtin.RawPointer):
+  return %z1 : $Builtin.RawPointer
+}
+
 // CHECK-LABEL: sil hidden @sink_allocation_inst
 sil hidden @sink_allocation_inst : $@convention(thin) (Boo, Boo) -> Bool {
 bb0(%0 : $Boo, %1 : $Boo):

test/SILOptimizer/simplify_cfg_dom_jumpthread.sil

@@ -558,3 +558,39 @@ bb6:
   fix_lifetime %6 : $E2
   unreachable
 }
+
+// CHECK-LABEL: sil [ossa] @test_update_borrowed_from :
+// CHECK:       bb3([[A1:%.*]] : @guaranteed $C, [[A2:%.*]] : @reborrow $C):
+// CHECK-DAG:     = borrowed [[A2]] : $C from ()
+// CHECK-DAG:     = borrowed [[A1]] : $C from ([[A2]] : $C)
+// CHECK:       } // end sil function 'test_update_borrowed_from'
+sil [ossa] @test_update_borrowed_from : $@convention(thin) (Builtin.Int1, @inout C) -> () {
+bb0(%0 : $Builtin.Int1, %1 : $*C):
+  cond_br %0, bb2, bb1
+
+bb1:
+  br bb3
+
+bb2:
+  %4 = load [take] %1
+  store %4 to [init] %1
+  br bb3
+
+bb3:
+  %7 = load_borrow %1
+  cond_br %0, bb5, bb4
+
+bb4:
+  br bb6(%7)
+
+bb5:
+  %10 = unchecked_ref_cast %7 to $C
+  br bb6(%10)
+
+bb6(%12 : @guaranteed $C):
+  %13 = borrowed %12 from (%7)
+  end_borrow %7
+  %15 = tuple ()
+  return %15
+}
+

test/Serialization/source-loc.swift

@@ -14,6 +14,6 @@ let _ = foo(x: 100)
 //CHECK: {{.*builtin "cmp_ult_Int64".*}} loc "{{.*}}def_source_loc.swift":3:11
 //CHECK: {{.*cond_br.*}} loc "{{.*}}def_source_loc.swift":3:11
 //CHECK: {{.*integer_literal.*}} 1, loc "{{.*}}def_source_loc.swift":7:12
-//CHECK: {{.*struct \$UInt64.*}} loc "{{.*}}def_source_loc.swift":7:12
 //CHECK: {{.*br.*}} loc "{{.*}}def_source_loc.swift":7:5
+//CHECK: {{.*struct \$UInt64.*}} loc "{{.*}}def_source_loc.swift":7:12
 //CHECK: {{.*return.*}} loc "{{.*}}def_source_loc.swift":8:2