LoopRotate: remove redundant phis after ssa-update

Fixes an ownership error

Author: eeckstein

lib/SILOptimizer/LoopTransforms/LoopRotate.cpp

@@ -46,7 +46,7 @@ static llvm::cl::opt<bool> RotateSingleBlockLoop("looprotate-single-block-loop",
 
 static bool rotateLoop(SILLoop *loop, DominanceInfo *domInfo,
                        SILLoopInfo *loopInfo, bool rotateSingleBlockLoops,
-                       SILBasicBlock *upToBB);
+                       SILBasicBlock *upToBB, SILPassManager *pm);
 
 /// Check whether all operands are loop invariant.
 static bool
@@ -151,7 +151,8 @@ static void mapOperands(SILInstruction *inst,
 static void updateSSAForUseOfValue(
     SILSSAUpdater &updater, SmallVectorImpl<SILPhiArgument *> &insertedPhis,
     const llvm::DenseMap<ValueBase *, SILValue> &valueMap,
-    SILBasicBlock *Header, SILBasicBlock *EntryCheckBlock, SILValue Res) {
+    SILBasicBlock *Header, SILBasicBlock *EntryCheckBlock, SILValue Res,
+    SILPassManager *pm) {
   // Find the mapped instruction.
   assert(valueMap.count(Res) && "Expected to find value in map!");
   SILValue MappedValue = valueMap.find(Res)->second;
@@ -187,23 +188,25 @@ static void updateSSAForUseOfValue(
     updater.rewriteUse(*use);
   }
 
+  replacePhisWithIncomingValues(pm, insertedPhis);
 }
 
 static void
 updateSSAForUseOfInst(SILSSAUpdater &updater,
                       SmallVectorImpl<SILPhiArgument *> &insertedPhis,
                       const llvm::DenseMap<ValueBase *, SILValue> &valueMap,
                       SILBasicBlock *header, SILBasicBlock *entryCheckBlock,
-                      SILInstruction *inst) {
+                      SILInstruction *inst, SILPassManager *pm) {
   for (auto result : inst->getResults())
     updateSSAForUseOfValue(updater, insertedPhis, valueMap, header,
-                           entryCheckBlock, result);
+                           entryCheckBlock, result, pm);
 }
 
 /// Rewrite the code we just created in the preheader and update SSA form.
 static void rewriteNewLoopEntryCheckBlock(
     SILBasicBlock *header, SILBasicBlock *entryCheckBlock,
-    const llvm::DenseMap<ValueBase *, SILValue> &valueMap) {
+    const llvm::DenseMap<ValueBase *, SILValue> &valueMap,
+    SILPassManager *pm) {
   SmallVector<SILPhiArgument *, 8> insertedPhis;
   SILSSAUpdater updater(&insertedPhis);
 
@@ -212,7 +215,7 @@ static void rewriteNewLoopEntryCheckBlock(
   for (unsigned i : range(header->getNumArguments())) {
     auto *arg = header->getArguments()[i];
     updateSSAForUseOfValue(updater, insertedPhis, valueMap, header,
-                           entryCheckBlock, arg);
+                           entryCheckBlock, arg, pm);
   }
 
   auto instIter = header->begin();
@@ -221,7 +224,7 @@ static void rewriteNewLoopEntryCheckBlock(
   while (instIter != header->end()) {
     auto &inst = *instIter;
     updateSSAForUseOfInst(updater, insertedPhis, valueMap, header,
-                          entryCheckBlock, &inst);
+                          entryCheckBlock, &inst, pm);
     ++instIter;
   }
 }
@@ -243,7 +246,7 @@ static void updateDomTree(DominanceInfo *domInfo, SILBasicBlock *preheader,
 }
 
 static bool rotateLoopAtMostUpToLatch(SILLoop *loop, DominanceInfo *domInfo,
-                                      SILLoopInfo *loopInfo) {
+                                      SILLoopInfo *loopInfo, SILPassManager *pm) {
   auto *latch = loop->getLoopLatch();
   if (!latch) {
     LLVM_DEBUG(llvm::dbgs()
@@ -253,11 +256,11 @@ static bool rotateLoopAtMostUpToLatch(SILLoop *loop, DominanceInfo *domInfo,
 
   bool didRotate = rotateLoop(
       loop, domInfo, loopInfo,
-      RotateSingleBlockLoop /* rotateSingleBlockLoops */, latch);
+      RotateSingleBlockLoop /* rotateSingleBlockLoops */, latch, pm);
 
   // Keep rotating at most until we hit the original latch.
   if (didRotate)
-    while (rotateLoop(loop, domInfo, loopInfo, false, latch)) {
+    while (rotateLoop(loop, domInfo, loopInfo, false, latch, pm)) {
     }
 
   return didRotate;
@@ -306,7 +309,7 @@ static bool isSingleBlockLoop(SILLoop *L) {
 /// loop for termination.
 static bool rotateLoop(SILLoop *loop, DominanceInfo *domInfo,
                        SILLoopInfo *loopInfo, bool rotateSingleBlockLoops,
-                       SILBasicBlock *upToBB) {
+                       SILBasicBlock *upToBB, SILPassManager *pm) {
   assert(loop != nullptr && domInfo != nullptr && loopInfo != nullptr
          && "Missing loop information");
 
@@ -432,7 +435,7 @@ static bool rotateLoop(SILLoop *loop, DominanceInfo *domInfo,
 
   // If there were any uses of instructions in the duplicated loop entry check
   // block rewrite them using the ssa updater.
-  rewriteNewLoopEntryCheckBlock(header, preheader, valueMap);
+  rewriteNewLoopEntryCheckBlock(header, preheader, valueMap, pm);
 
   loop->moveToHeader(newHeader);
 
@@ -498,7 +501,7 @@ class LoopRotation : public SILFunctionTransform {
         SILLoop *loop = worklist.pop_back_val();
         changed |= canonicalizeLoop(loop, domInfo, loopInfo);
         changed |=
-            rotateLoopAtMostUpToLatch(loop, domInfo, loopInfo);
+            rotateLoopAtMostUpToLatch(loop, domInfo, loopInfo, getPassManager());
       }
     }
 

test/SILOptimizer/looprotate_ossa.sil

@@ -582,3 +582,134 @@ bb3:
   return %26 : $Int32
 }
 
+// Make sure that no unnecessary phis are inserted which would cause an ownership error.
+// CHECK-LABEL: sil [ossa] @test_ssa_updater_owned :
+// CHECK:         begin_borrow
+// CHECK-NOT:   bb({{.*@owned.*}}):
+// CHECK:         end_borrow
+// CHECK-LABEL: } // end sil function 'test_ssa_updater_owned'
+sil [ossa] @test_ssa_updater_owned : $@convention(thin) (@owned String, @guaranteed String) -> () {
+bb0(%0 : @owned $String, %1 : @guaranteed $String):
+  %2 = integer_literal $Builtin.Int64, 0
+  br bb1(%0, %2)
+
+bb1(%4 : @owned $String, %5 : $Builtin.Int64):
+  cond_br undef, bb14, bb2
+
+bb2:
+  %7 = integer_literal $Builtin.Int64, 1
+  %8 = begin_borrow %4
+  cond_br undef, bb4, bb3
+
+bb3:
+  br bb5
+
+bb4:
+  cond_br undef, bb7, bb8
+
+bb5:
+  cond_br undef, bb10, bb6
+
+bb6:
+  br bb9
+
+bb7:
+  br bb9
+
+bb8:
+  br bb12
+
+bb9:
+  br bb11
+
+bb10:
+  br bb11
+
+bb11:
+  end_borrow %8
+  destroy_value %4
+  br bb13
+
+bb12:
+  end_borrow %8
+  destroy_value [dead_end] %4
+  unreachable 
+
+bb13:
+  %24 = copy_value %1
+  br bb1(%24, %7)
+
+bb14:
+  destroy_value %4
+  %27 = tuple ()
+  return %27
+}
+
+// Make sure that no unnecessary phis are inserted which would cause an ownership error.
+// CHECK-LABEL: sil [ossa] @test_ssa_updater_guaranteed :
+// CHECK:         begin_borrow
+// CHECK:         begin_borrow
+// CHECK-NOT:   bb({{.*@reborrow.*}}):
+// CHECK:         end_borrow
+// CHECK-LABEL: } // end sil function 'test_ssa_updater_guaranteed'
+sil [ossa] @test_ssa_updater_guaranteed : $@convention(thin) (@owned String) -> () {
+bb0(%0 : @owned $String):
+  %2 = integer_literal $Builtin.Int64, 0
+  %3 = begin_borrow %0
+  br bb1(%3, %2)
+
+bb1(%4 : @reborrow $String, %5 : $Builtin.Int64):
+  %6 = borrowed %4 from (%0)
+  cond_br undef, bb14, bb2
+
+bb2:
+  %7 = integer_literal $Builtin.Int64, 1
+  %8 = begin_borrow %6
+  cond_br undef, bb4, bb3
+
+bb3:
+  br bb5
+
+bb4:
+  cond_br undef, bb7, bb8
+
+bb5:
+  cond_br undef, bb10, bb6
+
+bb6:
+  br bb9
+
+bb7:
+  br bb9
+
+bb8:
+  br bb12
+
+bb9:
+  br bb11
+
+bb10:
+  br bb11
+
+bb11:
+  end_borrow %8
+  end_borrow %6
+  br bb13
+
+bb12:
+  end_borrow %8
+  end_borrow %6
+  destroy_value [dead_end] %0
+  unreachable 
+
+bb13:
+  %24 = begin_borrow %0
+  br bb1(%24, %7)
+
+bb14:
+  end_borrow %6
+  destroy_value %0
+  %27 = tuple ()
+  return %27
+}
+