- strict rules

Author: eeckstein

SwiftCompilerSources/Sources/Optimizer/InstructionSimplification/SimplifyBranch.swift

@@ -49,7 +49,15 @@ private extension BranchInst {
     let parentBB = parentBlock
 
     for (argIdx, op) in operands.enumerated() {
-      targetBB.arguments[argIdx].uses.replaceAll(with: op.value, context)
+      let arg = targetBB.arguments[argIdx]
+      if let phi = Phi(arg),
+         let bfi = phi.borrowedFrom
+      {
+        bfi.uses.replaceAll(with: op.value, context)
+        context.erase(instruction: bfi)
+      } else {
+        arg.uses.replaceAll(with: op.value, context)
+      }
     }
     targetBB.eraseAllArguments(context)
 

SwiftCompilerSources/Sources/Optimizer/InstructionSimplification/SimplifySwitchEnum.swift

@@ -48,12 +48,14 @@ extension SwitchEnumInst : OnoneSimplifyable {
       precondition(enumInst.payload == nil || !parentFunction.hasOwnership,
                    "missing payload argument in switch_enum case block")
       builder.createBranch(to: caseBlock)
+      context.erase(instruction: self)
     case 1:
       builder.createBranch(to: caseBlock, arguments: [enumInst.payload!])
+      context.erase(instruction: self)
+      updateBorrowedFrom(for: [Phi(caseBlock.arguments[0])!], context)
     default:
       fatalError("case block of switch_enum cannot have more than 1 argument")
     }
-    context.erase(instruction: self)
 
     if canEraseEnumInst {
       context.erase(instructionIncludingDebugUses: enumInst)

SwiftCompilerSources/Sources/Optimizer/Utilities/BorrowUtils.swift

@@ -534,7 +534,7 @@ func gatherEnclosingValues(for value: Value,
       // There is no enclosing value on this path.
       break
     case .reborrow(let phi):
-      enclosingValues.append(contentsOf: phi.borrowedFrom.enclosingValues)
+      enclosingValues.append(contentsOf: phi.borrowedFrom!.enclosingValues)
     }
   } else {
     // Handle forwarded guaranteed values.

SwiftCompilerSources/Sources/Optimizer/Utilities/BorrowedFromUpdater.swift

@@ -35,15 +35,19 @@ func updateBorrowedFrom(for phis: some Sequence<Phi>, _ context: some MutatingCo
     if !phi.value.parentFunction.hasOwnership {
       return
     }
-    createBorrowedFrom(for: phi, context)
+    if phi.value.ownership == .guaranteed {
+      createBorrowedFrom(for: phi, context)
+    }
   }
 
   var changed: Bool
   repeat {
     changed = false
 
     for phi in phis {
-      changed = updateBorrowedFrom(for: phi, context) || changed
+      if phi.value.ownership == .guaranteed {
+        changed = updateBorrowedFrom(for: phi, context) || changed
+      }
     }
   } while changed
 }

SwiftCompilerSources/Sources/Optimizer/Utilities/Verifier.swift

@@ -37,11 +37,10 @@ extension BorrowedFromInst : VerifyableInstruction {
     var computedEVs = Stack<Value>(context)
     defer { computedEVs.deinitialize() }
 
-    if let phi = Phi(borrowedValue) {
-      gatherEnclosingValuesFromPredecessors(for: phi, in: &computedEVs, context)
-    } else {
-      gatherEnclosingValues(for: borrowedValue, in: &computedEVs, context)
+    guard let phi = Phi(borrowedValue) else {
+      fatalError("borrowed value of borrowed-from must be a phi: \(self)")
     }
+    gatherEnclosingValuesFromPredecessors(for: phi, in: &computedEVs, context)
 
     var existingEVs = ValueSet(insertContentsOf: enclosingValues, context)
     defer { existingEVs.deinitialize() }
@@ -60,7 +59,8 @@ private extension Argument {
       for use in phi.value.uses {
         precondition(use.instruction is BorrowedFromInst,
                      "guaranteed phi \(self) has non borrowed-from use \(use)")
-        if use.forwardingBorrowedFromUser != nil {
+        if use.index == 0 {
+          precondition(!forwardingBorrowedFromFound, "phi \(self) has multiple forwarding borrowed-from uses")
           forwardingBorrowedFromFound = true
         }
       }

SwiftCompilerSources/Sources/SIL/Argument.swift

@@ -128,13 +128,13 @@ public struct Phi {
     value.ownership == .owned || value.isReborrow
   }
 
-  public var borrowedFrom: BorrowedFromInst {
+  public var borrowedFrom: BorrowedFromInst? {
     for use in value.uses {
       if let bfi = use.forwardingBorrowedFromUser {
         return bfi
       }
     }
-    fatalError("phi argument doesn't have a borrowed-from user")
+    return nil
   }
 
   public static func ==(lhs: Phi, rhs: Phi) -> Bool {

include/swift/SIL/OwnershipUtils.h

@@ -90,6 +90,7 @@ inline bool isForwardingConsume(SILValue value) {
 //                        Ownership Def-Use Utilities
 //===----------------------------------------------------------------------===//
 
+BorrowedFromInst *getBorrowedFromUser(SILValue v);
 SILValue lookThroughSingleBorrowedFromUser(SILValue v);
 SILValue lookThroughBorrowedFromDef(SILValue v);
 void getTransitiveConsumingUses(SILValue v, llvm::SmallVectorImpl<Operand *> &results);

include/swift/SIL/SILValue.h

@@ -1071,8 +1071,11 @@ class Operand {
     removeFromCurrent();
     TheValueAndThreeBits.setPointer(newValue);
     insertIntoCurrent();
+    verify();
   }
 
+  void verify() const;
+
   /// Swap the given operand with the current one.
   void swap(Operand &Op) {
     SILValue OtherV = Op.get();

lib/SIL/IR/SILValue.cpp

@@ -410,6 +410,12 @@ llvm::raw_ostream &swift::operator<<(llvm::raw_ostream &os,
 //                                  Operand
 //===----------------------------------------------------------------------===//
 
+void Operand::verify() const {
+  if (isa<BorrowedFromInst>(getUser()) && getOperandNumber() == 0) {
+    assert(isa<SILArgument>(get()) || isa<SILUndef>(get()));
+  }
+}
+
 SILBasicBlock *Operand::getParentBlock() const {
   auto *self = const_cast<Operand *>(this);
   return self->getUser()->getParent();

lib/SIL/Utils/BasicBlockUtils.cpp

@@ -16,6 +16,7 @@
 #include "swift/Basic/STLExtras.h"
 #include "swift/SIL/Dominance.h"
 #include "swift/SIL/LoopInfo.h"
+#include "swift/SIL/OwnershipUtils.h"
 #include "swift/SIL/SILArgument.h"
 #include "swift/SIL/SILBasicBlock.h"
 #include "swift/SIL/SILBuilder.h"
@@ -345,8 +346,14 @@ void swift::mergeBasicBlockWithSingleSuccessor(SILBasicBlock *BB,
 
   // If there are any BB arguments in the destination, replace them with the
   // branch operands, since they must dominate the dest block.
-  for (unsigned i = 0, e = BI->getArgs().size(); i != e; ++i)
-    succBB->getArgument(i)->replaceAllUsesWith(BI->getArg(i));
+  for (unsigned i = 0, e = BI->getArgs().size(); i != e; ++i) {
+    SILArgument *arg = succBB->getArgument(i);
+    if (auto *bfi = getBorrowedFromUser(arg)) {
+      bfi->replaceAllUsesWith(arg);
+      bfi->eraseFromParent();
+    }
+    arg->replaceAllUsesWith(BI->getArg(i));
+  }
 
   BI->eraseFromParent();
 

lib/SIL/Utils/OwnershipUtils.cpp

@@ -203,19 +203,20 @@ bool swift::computeIsGuaranteedForwarding(SILValue value) {
   return isGuaranteedForwardingPhi;
 }
 
-SILValue swift::lookThroughSingleBorrowedFromUser(SILValue v) {
-  BorrowedFromInst *singleBfi = nullptr;
+BorrowedFromInst *swift::getBorrowedFromUser(SILValue v) {
   for (auto *use : v->getUses()) {
     if (auto *bfi = dyn_cast<BorrowedFromInst>(use->getUser())) {
       if (use->getOperandNumber() == 0) {
-        if (singleBfi)
-          return v;
-        singleBfi = bfi;
+        return bfi;
       }
     }
   }
-  if (singleBfi)
-    return singleBfi;
+  return nullptr;
+}
+
+SILValue swift::lookThroughSingleBorrowedFromUser(SILValue v) {
+  if (BorrowedFromInst *bfi = getBorrowedFromUser(v))
+    return bfi;
   return v;
 }
 

lib/SILOptimizer/LoopTransforms/LoopRotate.cpp

@@ -410,7 +410,9 @@ bool swift::rotateLoop(SILLoop *loop, DominanceInfo *domInfo,
   }
 
   for (auto &inst : *header) {
-    if (SILInstruction *cloned = inst.clone(preheaderBranch)) {
+    if (auto *bfi = dyn_cast<BorrowedFromInst>(&inst)) {
+      valueMap[bfi] = valueMap[bfi->getBorrowedValue()];
+    } else if (SILInstruction *cloned = inst.clone(preheaderBranch)) {
       mapOperands(cloned, valueMap);
 
       // The actual operand will sort out which result idx to use.

lib/SILOptimizer/Transforms/SimplifyCFG.cpp

@@ -1287,7 +1287,13 @@ bool SimplifyCFG::simplifyBranchBlock(BranchInst *BI) {
     for (unsigned i = 0, e = BI->getArgs().size(); i != e; ++i) {
       assert(DestBB->getArgument(i) != BI->getArg(i));
       SILValue Val = BI->getArg(i);
-      DestBB->getArgument(i)->replaceAllUsesWith(Val);
+      SILValue arg = DestBB->getArgument(i);
+      if (auto *bfi = getBorrowedFromUser(arg)) {
+        bfi->replaceAllUsesWith(Val);
+        bfi->eraseFromParent();
+      } else {
+        arg->replaceAllUsesWith(Val);
+      }
       if (!isVeryLargeFunction) {
         if (auto *I = dyn_cast<SingleValueInstruction>(Val)) {
           // Replacing operands may trigger constant folding which then could
@@ -1545,8 +1551,12 @@ bool SimplifyCFG::simplifyCondBrBlock(CondBranchInst *BI) {
     // Erase in reverse order to pop each element as we go.
     for (unsigned i = destBB->getArguments().size(); i != 0;) {
       --i;
-      destBB->getArgument(i)->replaceAllUsesWith(
-        trampolineDest.newSourceBranchArgs[i]);
+      SILArgument *arg = destBB->getArgument(i);
+      if (auto *bfi = getBorrowedFromUser(arg)) {
+        bfi->replaceAllUsesWith(arg);
+        bfi->eraseFromParent();
+      }
+      arg->replaceAllUsesWith(trampolineDest.newSourceBranchArgs[i]);
       destBB->eraseArgument(i);
     }
   };
@@ -3809,6 +3819,10 @@ static void tryToReplaceArgWithIncomingValue(SILBasicBlock *BB, unsigned i,
   // An argument has one result value. We need to replace this with the *value*
   // of the incoming block(s).
   LLVM_DEBUG(llvm::dbgs() << "replace arg with incoming value:" << *A);
+  if (auto *bfi = getBorrowedFromUser(A)) {
+    bfi->replaceAllUsesWith(A);
+    bfi->eraseFromParent();
+  }
   A->replaceAllUsesWith(V);
 }
 

lib/SILOptimizer/Utils/CFGOptUtils.cpp

@@ -19,6 +19,7 @@
 #include "swift/SIL/SILArgument.h"
 #include "swift/SIL/SILBuilder.h"
 #include "swift/SIL/BasicBlockDatastructures.h"
+#include "swift/SIL/OwnershipUtils.h"
 #include "swift/SILOptimizer/Utils/InstOptUtils.h"
 #include "llvm/ADT/TinyPtrVector.h"
 
@@ -142,8 +143,12 @@ TermInst *swift::deleteEdgeValue(TermInst *branch, SILBasicBlock *destBlock,
 void swift::erasePhiArgument(SILBasicBlock *block, unsigned argIndex,
                              bool cleanupDeadPhiOps,
                              InstModCallbacks callbacks) {
-  assert(block->getArgument(argIndex)->isPhi()
-         && "Only should be used on phi arguments");
+  SILArgument *arg = block->getArgument(argIndex);
+  assert(arg->isPhi() && "Only should be used on phi arguments");
+  if (auto *bfi = getBorrowedFromUser(arg)) {
+    bfi->replaceAllUsesWith(arg);
+    bfi->eraseFromParent();
+  }
   block->eraseArgument(argIndex);
 
   // Determine the set of predecessors in case any predecessor has