[sil-combine] Address comments from post-commit review of cd7c9e9.

Author: gottesmm

lib/SILOptimizer/SILCombiner/SILCombinerCastVisitors.cpp

@@ -569,8 +569,14 @@ static bool canBeUsedAsCastDestination(SILValue value, CastInst *castInst,
 
 SILInstruction *SILCombiner::visitUnconditionalCheckedCastAddrInst(
     UnconditionalCheckedCastAddrInst *uccai) {
-  // Optimize the unconditional_checked_cast_addr in this pattern:
+
+  // Optimize the unconditional_checked_cast_addr in the following non-ossa/ossa
+  // pattern:
+  //
+  // Non-OSSA Pattern
   //
+  //   %value = ...
+  //   ...
   //   %box = alloc_existential_box $Error, $ConcreteError
   //   %a = project_existential_box $ConcreteError in %b : $Error
   //   store %value to %a : $*ConcreteError
@@ -581,33 +587,67 @@ SILInstruction *SILCombiner::visitUnconditionalCheckedCastAddrInst(
   //                                ConcreteError in %dest : $*ConcreteError
   //
   // to:
-  //   ...
+  //
   //   retain_value %value : $ConcreteError
+  //   ...
+  //   %box = alloc_existential_box $Error, $ConcreteError
+  //   %a = project_existential_box $ConcreteError in %b : $Error
+  //   store %value to %a : $*ConcreteError
+  //   %err = alloc_stack $Error
+  //   store %box to %err : $*Error
   //   destroy_addr %err : $*Error
   //   store %value to %dest $*ConcreteError
   //
-  // This lets the alloc_existential_box become dead and it can be removed in
-  // following optimizations.
+  // OSSA Pattern:
+  //
+  //   %value = ...
+  //   ...
+  //   %box = alloc_existential_box $Error, $ConcreteError
+  //   %a = project_existential_box $ConcreteError in %b : $Error
+  //   store %value to [init] %a : $*ConcreteError
+  //   %err = alloc_stack $Error
+  //   store %box to [init] %err : $*Error
+  //   %dest = alloc_stack $ConcreteError
+  //   unconditional_checked_cast_addr Error in %err : $*Error to
+  //                                ConcreteError in %dest : $*ConcreteError
+  //
+  // to:
+  //
+  //   %value_copy = copy_value %value
+  //   ...
+  //   %box = alloc_existential_box $Error, $ConcreteError
+  //   %a = project_existential_box $ConcreteError in %b : $Error
+  //   store %value to [init] %a : $*ConcreteError
+  //   %err = alloc_stack $Error
+  //   store %box to [init] %err : $*Error
+  //   destroy_addr %err : $*Error
+  //   store %value to %dest $*ConcreteError
+  //
+  // In both cases, this lets the alloc_existential_box become dead and it can
+  // be removed in other subsequent optimizations.
   SILValue val = getConcreteValueOfExistentialBoxAddr(uccai->getSrc(), uccai);
   while (auto *cvi = dyn_cast_or_null<CopyValueInst>(val))
     val = cvi->getOperand();
   if (canBeUsedAsCastDestination(val, uccai, DA)) {
     // We need to copy the value at its insertion point.
     {
-      auto *valInsertPt = val->getDefiningInsertionPoint();
-      if (!valInsertPt)
+      auto *nextInsertPt = val->getNextInstruction();
+      if (!nextInsertPt)
         return nullptr;
-      auto valInsertPtIter = valInsertPt->getIterator();
       // If our value is defined by an instruction (not an argument), we want to
       // insert the copy after that. Otherwise, we have an argument and we want
       // to insert the copy right at the beginning of the block.
-      if (val->getDefiningInstruction())
-        valInsertPtIter = std::next(valInsertPtIter);
-      SILBuilderWithScope builder(valInsertPtIter, Builder);
-      val = builder.emitCopyValueOperation(valInsertPtIter->getLoc(), val);
+      SILBuilderWithScope builder(nextInsertPt, Builder);
+      // We use an autogenerated location to ensure that if next is a
+      // terminator, we do not trip an assertion around mismatched debug info.
+      //
+      // FIXME: We should find a better way of solving this than losing location
+      // info!
+      auto loc = RegularLocation::getAutoGeneratedLocation();
+      val = builder.emitCopyValueOperation(loc, val);
     }
 
-    // Then we insert the destroy value/store at the cast location.
+    // Then we insert the destroy addr/store at the cast location.
     SILBuilderWithScope builder(uccai, Builder);
     SILLocation loc = uccai->getLoc();
     builder.createDestroyAddr(loc, uccai->getSrc());
@@ -816,17 +856,12 @@ visitCheckedCastAddrBranchInst(CheckedCastAddrBranchInst *CCABI) {
     // We need to insert the copy after the defining instruction of val or at
     // the top of the block if val is an argument.
     {
-      auto *valInsertPt = val->getDefiningInsertionPoint();
-      if (!valInsertPt)
+      auto *nextInsertPt = val->getNextInstruction();
+      if (!nextInsertPt)
         return nullptr;
-      auto valInsertPtIter = valInsertPt->getIterator();
-      // If our value is defined by an instruction (not an argument), we want to
-      // insert the copy after that. Otherwise, we have an argument and we want
-      // to insert the copy right at the beginning of the block.
-      if (val->getDefiningInstruction())
-        valInsertPtIter = std::next(valInsertPtIter);
-      SILBuilderWithScope builder(valInsertPtIter, Builder);
-      val = builder.emitCopyValueOperation(valInsertPtIter->getLoc(), val);
+      SILBuilderWithScope builder(nextInsertPt, Builder);
+      auto loc = RegularLocation::getAutoGeneratedLocation();
+      val = builder.emitCopyValueOperation(loc, val);
     }
 
     SILBuilderWithScope builder(CCABI, Builder);

lib/SILOptimizer/Utils/InstOptUtils.cpp

@@ -14,6 +14,7 @@
 #include "swift/AST/GenericSignature.h"
 #include "swift/AST/SemanticAttrs.h"
 #include "swift/AST/SubstitutionMap.h"
+#include "swift/Basic/SmallPtrSetVector.h"
 #include "swift/SIL/ApplySite.h"
 #include "swift/SIL/BasicBlockUtils.h"
 #include "swift/SIL/DebugUtils.h"
@@ -701,11 +702,9 @@ SILValue swift::
 getConcreteValueOfExistentialBox(AllocExistentialBoxInst *existentialBox,
                                   SILInstruction *ignoreUser) {
   StoreInst *singleStore = nullptr;
-  SmallVector<Operand *, 32> worklist;
-  SmallPtrSet<Operand *, 8> addedToWorklistPreviously;
+  SmallPtrSetVector<Operand *, 32> worklist;
   for (auto *use : getNonDebugUses(existentialBox)) {
-    worklist.push_back(use);
-    addedToWorklistPreviously.insert(use);
+    worklist.insert(use);
   }
 
   while (!worklist.empty()) {
@@ -722,8 +721,7 @@ getConcreteValueOfExistentialBox(AllocExistentialBoxInst *existentialBox,
       // Look through copy_value, begin_borrow
       for (SILValue result : user->getResults())
         for (auto *transitiveUse : result->getUses())
-          if (!addedToWorklistPreviously.insert(transitiveUse).second)
-            worklist.push_back(use);
+          worklist.insert(transitiveUse);
       break;
     case SILInstructionKind::ProjectExistentialBoxInst: {
       auto *projectedAddr = cast<ProjectExistentialBoxInst>(user);