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

to-do: we should eventually remove code motion of retain and release instructions and implement them as semantic ARC optimizations in OSSA.

Author: eeckstein

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);

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/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