Merge pull request #30319 from gottesmm/pr-9df4a4957e5ff72c67e090d3880070d8cd472328

[semantic-arc-opts] Follow ups to #30289

Author: gottesmm

Diff for: lib/SILOptimizer/Transforms/SemanticARCOpts.cpp

@@ -54,9 +54,13 @@ class LiveRange {
   /// A list of destroy_values of the live range.
   SmallVector<Operand *, 2> destroyingUses;
 
-  /// A list of forwarding instructions that forward our destroys ownership, but
-  /// that are also able to forward guaranteed ownership.
-  SmallVector<Operand *, 2> generalForwardingUses;
+  /// A list of forwarding instructions that forward owned ownership, but that
+  /// are also able to be converted to guaranteed ownership. If we are able to
+  /// eliminate this LiveRange due to it being from a guaranteed value, we must
+  /// flip the ownership of all of these instructions to guaranteed from owned.
+  ///
+  /// Corresponds to isOwnershipForwardingInst(...).
+  SmallVector<Operand *, 2> ownershipForwardingUses;
 
   /// Consuming uses that we were not able to understand as a forwarding
   /// instruction or a destroy_value. These must be passed a strongly control
@@ -80,8 +84,7 @@ class LiveRange {
   /// Semantically this implies that a value is never passed off as +1 to memory
   /// or another function implying it can be used everywhere at +0.
   HasConsumingUse_t
-  hasConsumingUse(FrozenMultiMap<SILPhiArgument *, OwnedValueIntroducer>
-                      *phiToIncomingValueMultiMap = nullptr) const;
+  hasUnknownConsumingUse(bool assumingFixedPoint = false) const;
 
   ArrayRef<Operand *> getDestroyingUses() const { return destroyingUses; }
 
@@ -113,8 +116,8 @@ class LiveRange {
 
   OwnedValueIntroducer getIntroducer() const { return introducer; }
 
-  ArrayRef<Operand *> getNonConsumingForwardingUses() const {
-    return generalForwardingUses;
+  ArrayRef<Operand *> getOwnershipForwardingUses() const {
+    return ownershipForwardingUses;
   }
 
   void convertOwnedGeneralForwardingUsesToGuaranteed();
@@ -185,7 +188,7 @@ LiveRange::DestroyingInstsRange LiveRange::getDestroyingInsts() const {
 
 LiveRange::LiveRange(SILValue value)
     : introducer(*OwnedValueIntroducer::get(value)), destroyingUses(),
-      generalForwardingUses(), unknownConsumingUses() {
+      ownershipForwardingUses(), unknownConsumingUses() {
   assert(introducer.value.getOwnershipKind() == ValueOwnershipKind::Owned);
 
   // We know that our silvalue produces an @owned value. Look through all of our
@@ -246,7 +249,7 @@ LiveRange::LiveRange(SILValue value)
 
     // Ok, this is a forwarding instruction whose ownership we can flip from
     // owned -> guaranteed.
-    generalForwardingUses.push_back(op);
+    ownershipForwardingUses.push_back(op);
 
     // If we have a non-terminator, just visit its users recursively to see if
     // the the users force the live range to be alive.
@@ -336,8 +339,8 @@ void LiveRange::insertEndBorrowsAtDestroys(
 }
 
 void LiveRange::convertOwnedGeneralForwardingUsesToGuaranteed() {
-  while (!generalForwardingUses.empty()) {
-    auto *i = generalForwardingUses.pop_back_val()->getUser();
+  while (!ownershipForwardingUses.empty()) {
+    auto *i = ownershipForwardingUses.pop_back_val()->getUser();
 
     // If this is a term inst, just convert all of its incoming values that are
     // owned to be guaranteed.
@@ -436,9 +439,8 @@ void LiveRange::convertArgToGuaranteed(DeadEndBlocks &deadEndBlocks,
   convertOwnedGeneralForwardingUsesToGuaranteed();
 }
 
-LiveRange::HasConsumingUse_t LiveRange::hasConsumingUse(
-    FrozenMultiMap<SILPhiArgument *, OwnedValueIntroducer>
-        *phiToIncomingValueMultiMap) const {
+LiveRange::HasConsumingUse_t
+LiveRange::hasUnknownConsumingUse(bool assumingAtFixPoint) const {
   // First do a quick check if we have /any/ unknown consuming
   // uses. If we do not have any, return false early.
   if (unknownConsumingUses.empty()) {
@@ -447,7 +449,7 @@ LiveRange::HasConsumingUse_t LiveRange::hasConsumingUse(
 
   // Ok, we do have some unknown consuming uses. If we aren't asked to
   // update phiToIncomingValueMultiMap, then just return true quickly.
-  if (!phiToIncomingValueMultiMap) {
+  if (!assumingAtFixPoint) {
     return HasConsumingUse_t::Yes;
   }
 
@@ -828,7 +830,6 @@ static bool canEliminatePhi(
           if (!introducer.isConvertableToGuaranteed()) {
             return false;
           }
-
           // If this linear search is too slow, we can change the
           // multimap to sort the mapped to list by pointer
           // instead of insertion order. In such a case, we could
@@ -870,17 +871,17 @@ bool SemanticARCOptVisitor::performPostPeepholeOwnedArgElimination() {
     // First compute the LiveRange for our phi argument. For simplicity, we only
     // handle cases now where our phi argument does not have any phi unknown
     // consumers.
-    SILPhiArgument *phiArg = pair.first;
-    LiveRange phiArgLiveRange(phiArg);
-    if (bool(phiArgLiveRange.hasConsumingUse())) {
+    SILPhiArgument *phi = pair.first;
+    LiveRange phiLiveRange(phi);
+    if (bool(phiLiveRange.hasUnknownConsumingUse())) {
       continue;
     }
 
     // Ok, we know that our phi argument /could/ be converted to guaranteed if
     // our incoming values are able to be converted to guaranteed. Now for each
     // incoming value, compute the incoming values ownership roots and see if
     // all of the ownership roots are in our owned incoming value array.
-    if (!phiArg->getIncomingPhiOperands(incomingValueOperandList)) {
+    if (!phi->getIncomingPhiOperands(incomingValueOperandList)) {
       continue;
     }
 
@@ -903,7 +904,7 @@ bool SemanticARCOptVisitor::performPostPeepholeOwnedArgElimination() {
     for (Operand *incomingValueOperand : incomingValueOperandList) {
       originalIncomingValues.push_back(incomingValueOperand->get());
       SILType type = incomingValueOperand->get()->getType();
-      auto *undef = SILUndef::get(type, *phiArg->getFunction());
+      auto *undef = SILUndef::get(type, *phi->getFunction());
       incomingValueOperand->set(undef);
     }
 
@@ -956,7 +957,7 @@ bool SemanticARCOptVisitor::performPostPeepholeOwnedArgElimination() {
     }
 
     // Then convert the phi's live range to be guaranteed.
-    std::move(phiArgLiveRange)
+    std::move(phiLiveRange)
         .convertArgToGuaranteed(getDeadEndBlocks(), lifetimeFrontier,
                                 getCallbacks());
 
@@ -980,7 +981,7 @@ bool SemanticARCOptVisitor::performPostPeepholeOwnedArgElimination() {
 
     madeChange = true;
     if (VerifyAfterTransform) {
-      phiArg->getFunction()->verify();
+      phi->getFunction()->verify();
     }
   }
 
@@ -1175,9 +1176,9 @@ bool SemanticARCOptVisitor::performGuaranteedCopyValueOptimization(CopyValueInst
   // forwarding or a user that truly represents a necessary consume of the value
   // (e.x. storing into memory).
   LiveRange lr(cvi);
-  auto hasConsumingUseState =
-      lr.hasConsumingUse(getPhiToIncomingValueMultiMap());
-  if (hasConsumingUseState == LiveRange::HasConsumingUse_t::Yes) {
+  auto hasUnknownConsumingUseState =
+      lr.hasUnknownConsumingUse(getPhiToIncomingValueMultiMap());
+  if (hasUnknownConsumingUseState == LiveRange::HasConsumingUse_t::Yes) {
     return false;
   }
 
@@ -1286,7 +1287,8 @@ bool SemanticARCOptVisitor::performGuaranteedCopyValueOptimization(CopyValueInst
   // was consumed, the hasConsumedUse code updated phiToIncomingValueMultiMap
   // for us before returning its prognosis. After we reach a fixed point, we
   // will try to eliminate this value then.
-  if (hasConsumingUseState == LiveRange::HasConsumingUse_t::YesButAllPhiArgs) {
+  if (hasUnknownConsumingUseState ==
+      LiveRange::HasConsumingUse_t::YesButAllPhiArgs) {
     auto *op = lr.getSingleUnknownConsumingUse();
     assert(op);
     unsigned opNum = op->getOperandNumber();
@@ -1302,7 +1304,7 @@ bool SemanticARCOptVisitor::performGuaranteedCopyValueOptimization(CopyValueInst
 
       auto *arg = succBlock->getSILPhiArguments()[opNum];
       LiveRange phiArgLR(arg);
-      if (bool(phiArgLR.hasConsumingUse())) {
+      if (bool(phiArgLR.hasUnknownConsumingUse())) {
         return false;
       }
 
@@ -1784,7 +1786,7 @@ bool SemanticARCOptVisitor::visitLoadInst(LoadInst *li) {
   // -> load_borrow if we can put a copy_value on a cold path and thus
   // eliminate RR traffic on a hot path.
   LiveRange lr(li);
-  if (bool(lr.hasConsumingUse()))
+  if (bool(lr.hasUnknownConsumingUse()))
     return false;
 
   // Then check if our address is ever written to. If it is, then we cannot use