[semantic-arc] Prevent future pointer invalidation issues in the OwnedToGuaranteedPhiOp transform.

The specific problem here is that I am going to be adding some code to
SemanticARCOpts that eliminates reborrows and may need to create new phi
arguments and thus add arguments to edges. The weird thing about this is that
doing so actually requires us to create a new terminator!

This means that subtle pointer invalidation issues can occur here. To work
around that we store our terminators as SILBasicBlock, operand number since we
can always immediately find a terminator from its basic block. If we do not have
a terminator, we keep on just storing the SILInstruction itself.

NOTE: This only saves us from additive changes. Deletions are still an issue.

Author: gottesmm

lib/SILOptimizer/SemanticARC/Context.h

@@ -64,7 +64,44 @@ struct LLVM_LIBRARY_VISIBILITY Context {
   /// our LiveRange can not see through joined live ranges, we know that we
   /// should only be able to have a single owned value introducer for each
   /// consumed operand.
-  FrozenMultiMap<SILValue, Operand *> joinedOwnedIntroducerToConsumedOperands;
+  ///
+  /// NOTE: To work around potential invalidation of our consuming operands when
+  /// adding values to edges on the CFG, we store our Operands as a
+  /// SILBasicBlock and an operand number. We only add values to edges and never
+  /// remove/modify edges so the operand number should be safe.
+  struct ConsumingOperandState {
+    PointerUnion<SILBasicBlock *, SILInstruction *> parent;
+    unsigned operandNumber;
+
+    ConsumingOperandState() : parent(nullptr), operandNumber(UINT_MAX) {}
+
+    ConsumingOperandState(Operand *op)
+        : parent(), operandNumber(op->getOperandNumber()) {
+      if (auto *ti = dyn_cast<TermInst>(op->getUser())) {
+        parent = ti->getParent();
+      } else {
+        parent = op->getUser();
+      }
+    }
+
+    ConsumingOperandState(const ConsumingOperandState &other) :
+        parent(other.parent), operandNumber(other.operandNumber) {}
+
+    ConsumingOperandState &operator=(const ConsumingOperandState &other) {
+      parent = other.parent;
+      operandNumber = other.operandNumber;
+      return *this;
+    }
+
+    ~ConsumingOperandState() = default;
+
+    operator bool() const {
+      return bool(parent) && operandNumber != UINT_MAX;
+    }
+  };
+
+  FrozenMultiMap<SILValue, ConsumingOperandState>
+      joinedOwnedIntroducerToConsumedOperands;
 
   /// If set to true, then we should only run cheap optimizations that do not
   /// build up data structures or analyze code in depth.

lib/SILOptimizer/SemanticARC/CopyValueOpts.cpp

@@ -224,9 +224,9 @@ bool SemanticARCOptVisitor::performGuaranteedCopyValueOptimization(
     });
 
     if (canOptimizePhi) {
+      Context::ConsumingOperandState state(opPhi);
       opPhi.visitResults([&](SILValue value) {
-        ctx.joinedOwnedIntroducerToConsumedOperands.insert(value,
-                                                           opPhi.getOperand());
+        ctx.joinedOwnedIntroducerToConsumedOperands.insert(value, state);
         return true;
       });
     }

lib/SILOptimizer/SemanticARC/OwnedToGuaranteedPhiOpt.cpp

@@ -19,12 +19,18 @@
 #include "Context.h"
 #include "OwnershipPhiOperand.h"
 #include "Transforms.h"
+#include "swift/Basic/STLExtras.h"
 
 using namespace swift;
 using namespace swift::semanticarc;
 
+namespace {
+using ConsumingOperandState = Context::ConsumingOperandState;
+} // anonymous namespace
+
+template <typename OperandRangeTy>
 static bool canEliminatePhi(
-    Context::FrozenMultiMapRange optimizableIntroducerRange,
+    OperandRangeTy optimizableIntroducerRange,
     ArrayRef<OwnershipPhiOperand> incomingValueOperandList,
     SmallVectorImpl<OwnedValueIntroducer> &ownedValueIntroducerAccumulator) {
   for (auto incomingValueOperand : incomingValueOperandList) {
@@ -161,9 +167,19 @@ bool swift::semanticarc::tryConvertOwnedPhisToGuaranteedPhis(Context &ctx) {
     // eliminated if it was not for the given phi. If all of them are, we can
     // optimize!
     {
-      auto rawFoundOptimizableIntroducerArray = pair.second;
-      if (!canEliminatePhi(rawFoundOptimizableIntroducerArray,
-                           incomingValueOperandList, ownedValueIntroducers)) {
+      std::function<Operand *(const Context::ConsumingOperandState)> lambda =
+        [&](const Context::ConsumingOperandState &state) -> Operand * {
+            unsigned opNum = state.operandNumber;
+            if (state.parent.is<SILBasicBlock *>()) {
+              SILBasicBlock *block = state.parent.get<SILBasicBlock *>();
+              return &block->getTerminator()->getAllOperands()[opNum];
+            }
+            SILInstruction *inst = state.parent.get<SILInstruction *>();
+            return &inst->getAllOperands()[opNum];
+        };
+      auto operandsTransformed = makeTransformRange(pair.second, lambda);
+      if (!canEliminatePhi(operandsTransformed, incomingValueOperandList,
+                           ownedValueIntroducers)) {
         continue;
       }
     }

lib/SILOptimizer/SemanticARC/OwnershipPhiOperand.h

@@ -64,6 +64,9 @@ class LLVM_LIBRARY_VISIBILITY OwnershipPhiOperand {
     }
   }
 
+  operator const Operand *() const { return op; }
+  operator Operand *() { return op; }
+
   Operand *getOperand() const { return op; }
   SILValue getValue() const { return op->get(); }
   SILType getType() const { return op->get()->getType(); }