Improve the SILPhiArgument API

This subclass of SILArgument should be eliminated--it's not always a
phi, and whether it is a "phi argument" has nothing whatsoever to do
with the opcode. That is a property of a value's uses, not a property of the
value.

Until then, provide a logical and useful API within the type. This
often avoids the need to explicitly cast to a SILPhiArgument type and
avoids a lot of boilerplate in code that deals with phis.

Note: PhiOperand and PhiValue are improved abstractions on top of this
API. But the SILArgument-level API is still an important bridge
between SILArgument and other phi abstractions.

Author: atrick

include/swift/SIL/MemAccessUtils.h

@@ -1689,7 +1689,7 @@ Result AccessUseDefChainVisitor<Impl, Result>::visit(SILValue sourceAddr) {
 
   case ValueKind::SILPhiArgument: {
     auto *phiArg = cast<SILPhiArgument>(sourceAddr);
-    if (phiArg->isPhiArgument()) {
+    if (phiArg->isPhi()) {
       return asImpl().visitPhi(phiArg);
     }
 

include/swift/SIL/SILArgument.h

@@ -22,6 +22,7 @@ namespace swift {
 
 class SILBasicBlock;
 class SILModule;
+class SILPhiArgument;
 class SILUndef;
 class TermInst;
 
@@ -115,9 +116,15 @@ class SILArgument : public ValueBase {
 
   unsigned getIndex() const;
 
-  /// Return true if this block argument is actually a phi argument as
-  /// opposed to a cast or projection.
-  bool isPhiArgument() const;
+  /// Return non-null if \p value is a phi.
+  static SILPhiArgument *isPhi(SILValue value);
+
+  /// Return non-null if \p value is a terminator result.
+  static SILPhiArgument *isTerminatorResult(SILValue value);
+
+  /// Return true if this block argument is a phi as opposed to a terminator
+  /// result.
+  bool isPhi() const;
 
   /// Return true if this block argument is a terminator result.
   bool isTerminatorResult() const;
@@ -217,12 +224,12 @@ class SILPhiArgument : public SILArgument {
       : SILArgument(ValueKind::SILPhiArgument, type, ownershipKind, decl) {}
 
 public:
-  /// Return true if this is block argument is actually a phi argument as
-  /// opposed to a cast or projection.
-  bool isPhiArgument() const;
+  /// Return true if this is block argument is a phi, as opposed to a terminator
+  /// result.
+  bool isPhi() const;
 
   /// Return true if this block argument is a terminator result.
-  bool isTerminatorResult() const { return !isPhiArgument(); }
+  bool isTerminatorResult() const { return !isPhi(); }
 
   /// If this argument is a phi, return the incoming phi value for the given
   /// predecessor BB. If this argument is not a phi, return an invalid SILValue.
@@ -365,16 +372,34 @@ class SILFunctionArgument : public SILArgument {
 // Out of line Definitions for SILArgument to avoid Forward Decl issues
 //===----------------------------------------------------------------------===//
 
-inline bool SILArgument::isPhiArgument() const {
+/// Return non-null if \p value is a real phi argument.
+inline SILPhiArgument *SILArgument::isPhi(SILValue value) {
+  if (auto *arg = dyn_cast<SILPhiArgument>(value)) {
+    if (arg->isPhi())
+      return arg;
+  }
+  return nullptr;
+}
+
+inline bool SILArgument::isPhi() const {
   switch (getKind()) {
   case SILArgumentKind::SILPhiArgument:
-    return cast<SILPhiArgument>(this)->isPhiArgument();
+    return cast<SILPhiArgument>(this)->isPhi();
   case SILArgumentKind::SILFunctionArgument:
     return false;
   }
   llvm_unreachable("Covered switch is not covered?!");
 }
 
+/// Return non-null if \p value is a terminator result.
+inline SILPhiArgument *SILArgument::isTerminatorResult(SILValue value) {
+  if (auto *arg = dyn_cast<SILPhiArgument>(value)) {
+    if (arg->isTerminatorResult())
+      return arg;
+  }
+  return nullptr;
+}
+
 inline bool SILArgument::isNoImplicitCopy() const {
   if (auto *fArg = dyn_cast<SILFunctionArgument>(this))
     return fArg->isNoImplicitCopy();

include/swift/SIL/SILBasicBlock.h

@@ -581,7 +581,7 @@ struct PhiValue {
 
   PhiValue(SILValue value) {
     auto *blockArg = dyn_cast<SILPhiArgument>(value);
-    if (!blockArg || !blockArg->isPhiArgument())
+    if (!blockArg || !blockArg->isPhi())
       return;
 
     phiBlock = blockArg->getParent();
@@ -600,6 +600,11 @@ struct PhiValue {
     return cast<SILPhiArgument>(phiBlock->getArgument(argIndex));
   }
 
+  Operand *getOperand(SILBasicBlock *predecessor) {
+    auto *branch = cast<BranchInst>(predecessor->getTerminator());
+    return &branch->getAllOperands()[argIndex];
+  }
+
   operator SILValue() const { return getValue(); }
   SILValue operator*() const { return getValue(); }
   SILValue operator->() const { return getValue(); }

lib/SIL/IR/SILArgument.cpp

@@ -82,7 +82,7 @@ SILParameterInfo SILFunctionArgument::getKnownParameterInfo() const {
 // expensive to call this helper instead of simply specifying phis with an
 // opcode. It results in repeated CFG traversals and repeated, unnecessary
 // switching over terminator opcodes.
-bool SILPhiArgument::isPhiArgument() const {
+bool SILPhiArgument::isPhi() const {
   // No predecessors indicates an unreachable block.
   if (getParent()->pred_empty())
     return false;
@@ -128,7 +128,7 @@ static SILValue getIncomingPhiValueForPred(const SILBasicBlock *parentBlock,
 }
 
 SILValue SILPhiArgument::getIncomingPhiValue(SILBasicBlock *predBlock) const {
-  if (!isPhiArgument())
+  if (!isPhi())
     return SILValue();
 
   const auto *parentBlock = getParent();
@@ -145,7 +145,7 @@ SILValue SILPhiArgument::getIncomingPhiValue(SILBasicBlock *predBlock) const {
 
 bool SILPhiArgument::getIncomingPhiValues(
     SmallVectorImpl<SILValue> &returnedPhiValues) const {
-  if (!isPhiArgument())
+  if (!isPhi())
     return false;
 
   const auto *parentBlock = getParent();
@@ -162,14 +162,14 @@ bool SILPhiArgument::getIncomingPhiValues(
 }
 
 Operand *SILPhiArgument::getIncomingPhiOperand(SILBasicBlock *predBlock) const {
-  if (!isPhiArgument())
+  if (!isPhi())
     return nullptr;
   return getIncomingPhiOperandForPred(getParent(), predBlock, getIndex());
 }
 
 bool SILPhiArgument::getIncomingPhiOperands(
     SmallVectorImpl<Operand *> &returnedPhiOperands) const {
-  if (!isPhiArgument())
+  if (!isPhi())
     return false;
 
   const auto *parentBlock = getParent();
@@ -187,7 +187,7 @@ bool SILPhiArgument::getIncomingPhiOperands(
 
 bool SILPhiArgument::visitIncomingPhiOperands(
     function_ref<bool(Operand *)> visitor) const {
-  if (!isPhiArgument())
+  if (!isPhi())
     return false;
 
   const auto *parentBlock = getParent();
@@ -210,7 +210,7 @@ bool SILPhiArgument::visitIncomingPhiOperands(
 bool SILPhiArgument::getIncomingPhiValues(
     SmallVectorImpl<std::pair<SILBasicBlock *, SILValue>>
         &returnedPredBBAndPhiValuePairs) const {
-  if (!isPhiArgument())
+  if (!isPhi())
     return false;
 
   const auto *parentBlock = getParent();
@@ -318,8 +318,10 @@ TermInst *SILPhiArgument::getSingleTerminator() const {
 
 TermInst *SILPhiArgument::getTerminatorForResult() const {
   if (auto *termInst = getSingleTerminator()) {
-    if (!isa<BranchInst>(termInst) && !isa<CondBranchInst>(termInst))
+    if (!swift::isa<BranchInst>(termInst)
+        && !swift::isa<CondBranchInst>(termInst)) {
       return termInst;
+    }
   }
   return nullptr;
 }

lib/SIL/IR/SILBasicBlock.cpp

@@ -401,7 +401,7 @@ bool SILBasicBlock::hasPhi() const {
   // It is sufficient to check whether the first argument is a phi.  A block
   // can't have both phis and terminator results.
   auto *argument = getArguments()[0];
-  return argument->isPhiArgument();
+  return argument->isPhi();
 }
 
 const SILDebugScope *SILBasicBlock::getScopeOfFirstNonMetaInstruction() {

lib/SIL/Utils/MemAccessUtils.cpp

@@ -789,7 +789,7 @@ class FindReferenceRoot {
       ref = svi->getOperand(0);
     };
     auto *phi = dyn_cast<SILPhiArgument>(ref);
-    if (!phi || !phi->isPhiArgument()) {
+    if (!phi || !phi->isPhi()) {
       return ref;
     }
     // Handle phis...
@@ -1533,7 +1533,7 @@ class AccessPathDefUseTraversal {
   void initializeDFS(SILValue root) {
     // If root is a phi, record it so that its uses aren't visited twice.
     if (auto *phi = dyn_cast<SILPhiArgument>(root)) {
-      if (phi->isPhiArgument())
+      if (phi->isPhi())
         visitedPhis.insert(phi);
     }
     pushUsers(root,

lib/SIL/Verifier/SILVerifier.cpp

@@ -1003,7 +1003,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     // Verify that the `isPhiArgument` property is sound:
     // - Phi arguments come from branches.
     // - Non-phi arguments have a single predecessor.
-    assert(arg->isPhiArgument() && "precondition");
+    assert(arg->isPhi() && "precondition");
     for (SILBasicBlock *predBB : arg->getParent()->getPredecessorBlocks()) {
       auto *TI = predBB->getTerminator();
       if (F.hasOwnership()) {
@@ -1015,7 +1015,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
                 "All phi argument inputs must be from branches.");
       }
     }
-    if (arg->isPhiArgument() && prohibitAddressPhis()) {
+    if (arg->isPhi() && prohibitAddressPhis()) {
       // As a property of well-formed SIL, we disallow address-type
       // phis. Supporting them would prevent reliably reasoning about the
       // underlying storage of memory access. This reasoning is important for
@@ -1033,7 +1033,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     checkLegalType(arg->getFunction(), arg, nullptr);
     checkValueBaseOwnership(arg);
     if (auto *phiArg = dyn_cast<SILPhiArgument>(arg)) {
-      if (phiArg->isPhiArgument())
+      if (phiArg->isPhi())
         visitSILPhiArgument(phiArg);
       else {
         // A non-phi BlockArgument must have a single predecessor unless it is

lib/SILOptimizer/Transforms/DeadCodeElimination.cpp

@@ -575,8 +575,8 @@ bool DCE::removeDead() {
         continue;
       }
 
-      if (!arg->isPhiArgument()) {
-        // We cannot delete a non phi arg. If it was @owned, insert a
+      if (!arg->isPhi()) {
+        // We cannot delete a non phi. If it was @owned, insert a
         // destroy_value, because its consuming user has already been marked
         // dead and will be deleted.
         // We do not have to end lifetime of a @guaranteed non phi arg.

lib/SILOptimizer/Transforms/PhiArgumentOptimizations.cpp

@@ -106,7 +106,7 @@ static bool hasOnlyNoneOwnershipIncomingValues(SILPhiArgument *phi) {
       if (incomingValue.getOwnershipKind() == OwnershipKind::None)
         continue;
       if (auto *incomingPhi = dyn_cast<SILPhiArgument>(incomingValue)) {
-        if (incomingPhi->isPhiArgument()) {
+        if (incomingPhi->isPhi()) {
           worklist.insert(incomingPhi);
           continue;
         }
@@ -153,7 +153,7 @@ bool RedundantPhiEliminationPass::optimizeArgs(SILBasicBlock *block) {
 
       SILArgument *arg1 = block->getArgument(arg1Idx);
       SILArgument *arg2 = block->getArgument(arg2Idx);
-      if (!arg1->isPhiArgument() || !arg2->isPhiArgument())
+      if (!arg1->isPhi() || !arg2->isPhi())
         continue;
 
       if (valuesAreEqual(arg1, arg2)) {
@@ -343,7 +343,7 @@ void PhiExpansionPass::run() {
   bool changed = false;
   for (SILBasicBlock &block : *getFunction()) {
     for (auto argAndIdx : enumerate(block.getArguments())) {
-      if (!argAndIdx.value()->isPhiArgument())
+      if (!argAndIdx.value()->isPhi())
         continue;
 
       unsigned idx = argAndIdx.index();

lib/SILOptimizer/Transforms/SimplifyCFG.cpp

@@ -1324,7 +1324,7 @@ TrampolineDest::TrampolineDest(SILBasicBlock *sourceBB,
   for (SILValue branchArg : targetBranch->getArgs()) {
     if (branchArg->getParentBlock() == targetBB) {
       auto *phi = dyn_cast<SILPhiArgument>(branchArg);
-      if (!phi || !phi->isPhiArgument()) {
+      if (!phi || !phi->isPhi()) {
         return;
       }
       branchArg = phi->getIncomingPhiValue(sourceBB);