Merge pull request #35563 from gottesmm/pr-cd7c9e97e46a0141e37f10fbfb74c094e0576ff3-followups

[ownership] cd7c9e9 followups

Author: gottesmm

include/swift/Basic/SmallPtrSetVector.h

@@ -0,0 +1,52 @@
+//===--- SmallPtrSetVector.h ----------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2021 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef SWIFT_BASIC_SMALLPTRSETVECTOR_H
+#define SWIFT_BASIC_SMALLPTRSETVECTOR_H
+
+#include "swift/Basic/LLVM.h"
+#include "llvm/ADT/SetVector.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/ADT/SmallVector.h"
+
+namespace swift {
+
+/// A SetVector that performs no allocations if smaller than a certain
+/// size. Uses a SmallPtrSet/SmallVector internally.
+template <typename T, unsigned VectorSize, unsigned SetSize = VectorSize>
+class SmallPtrSetVector : public llvm::SetVector<T, SmallVector<T, VectorSize>,
+                                                 SmallPtrSet<T, SetSize>> {
+public:
+  SmallPtrSetVector() = default;
+
+  /// Initialize a SmallPtrSetVector with a range of elements
+  template <typename It> SmallPtrSetVector(It Start, It End) {
+    this->insert(Start, End);
+  }
+};
+
+} // namespace swift
+
+namespace std {
+
+/// Implement std::swap in terms of SmallSetVector swap.
+///
+/// This matches llvm's implementation for SmallSetVector.
+template <typename T, unsigned VectorSize, unsigned SetSize = VectorSize>
+inline void swap(swift::SmallPtrSetVector<T, VectorSize, SetSize> &LHS,
+                 swift::SmallPtrSetVector<T, VectorSize, SetSize> &RHS) {
+  LHS.swap(RHS);
+}
+
+} // end namespace std
+
+#endif

lib/SIL/IR/SILValue.cpp

@@ -83,7 +83,7 @@ SILInstruction *ValueBase::getDefiningInsertionPoint() {
 
 SILInstruction *ValueBase::getNextInstruction() {
   if (auto *inst = getDefiningInstruction())
-    return std::next(inst);
+    return &*std::next(inst->getIterator());
   if (auto *arg = dyn_cast<SILArgument>(this))
     return &*arg->getParentBlock()->begin();
   return nullptr;

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