[Attributor][WIP] Implement AAReachability

[okura]

Summary

Currently, AAReachability does not compute anything and only use isPotentiallyReachable.
This patch makes it possible for AAReachability to use liveness information from AAIsDead.
In AAReachability::updateImpl, we collect reachable successor basic blocks for each basic block and construct a graph.
When reachability between two instructions is queried, check it using the graph.

[jdoerfert]

llvm/test/Transforms/Attributor/noalias.ll

919

Make an NFC pre-commit with the tests and "old" check lines. That makes it clear in this patch what changed. Just commit the test without additional review.

[okura]

• add missing override
• add tests in another commit

[jdoerfert]

Thanks for working on this! Reachability should be used more and with an actual optimistic implementation it makes sense. The next user (in addition to AANoAlias) could be value simplification, for load/store simplification. Similarly to the noalias case we can preserve other properties for a program point if it is not reachable from all potential violations (I am thinking a use of a dereferenceable argument that cannot be reached from a function call that is not nofree. This makes sense once we split dereferenceable_globally from dereferenceable.) Let's get this in first though ;)

Below some comments.

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

3262

This seems odd. If there is no reason for this version let's remove both initialize methods.

3267

Also not needed as far as I can tell.

3281–3284

3333

This looks like the AAIsDead code, right? Maybe we can instead expose some edge based interface, e.g., AAIsDead::isEdgeDead(BasicBlock, BasicBlock) or AAIsDead::getLiveOutEdges(BasicBlock). WDYT?

3375

We need to profile this but similar maps were causing memory problems before. Make the value a std::unique_ptr and that should already help. I'm not sure about this cache in general. I guess it mainly depends on the usage of this AA later on. For now we can keep it but we should invest in some time tracking for AAs at some point and thereby determine if this is too costly. If that is the case we could "cache" only the queries that were asked and recompute them in every update, not the entire graph.

[okura]

• reimplement updateImple and interfaces

[okura]

• fix according to interface change

[jdoerfert]

llvm/include/llvm/Transforms/IPO/Attributor.h

805

Why don't these life in the AAReachabilityFunction?

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

2226

Remove this and the initialize in the Impl class.

2232

typo above.

2235

Define this as EdgeTy or similar in the class once you moved the query vectors there.

2274

Don't use count above but the second element of the return value of insert.

[okura]

llvm/include/llvm/Transforms/IPO/Attributor.h

805

I put these here for the same reason for this.

[okura]

address comments

llvm/include/llvm/Transforms/IPO/Attributor.h

private:
   /// Attributor::identifyDefaultAbstractAttributes(...) can initialize them.
   friend struct Attributor;
 };
 
 /// The fixpoint analysis framework that orchestrates the attribute deduction.
 ///
 /// The Attributor provides a general abstract analysis framework (guided
 /// fixpoint iteration) as well as helper functions for the deduction of
 /// (LLVM-IR) attributes. However, also other code properties can be deduced,
 /// propagated, and ultimately manifested through the Attributor framework. This
 /// is particularly useful if these properties interact with attributes and a
 /// co-scheduled deduction allows to improve the solution. Even if not, thus if
 /// attributes/properties are completely isolated, they should use the
 /// Attributor framework to reduce the number of fixpoint iteration frameworks
 /// in the code base. Note that the Attributor design makes sure that isolated
 /// attributes are not impacted, in any way, by others derived at the same time
 /// if there is no cross-reasoning performed.
struct AAUndefinedBehavior
   static const char ID;
 };
 
 /// An abstract interface to determine reachability of point A to B.
 struct AAReachability : public StateWrapper<BooleanState, AbstractAttribute> {
   using Base = StateWrapper<BooleanState, AbstractAttribute>;
   AAReachability(const IRPosition &IRP, Attributor &A) : Base(IRP) {}
 
+  /// Returns true if 'From' instruction is assumed to reach 'To' instruction.
+  virtual bool getAssumedReachable(const Instruction &From,
+                                   const Instruction &To) const = 0;
+
   /// Returns true if 'From' instruction is assumed to reach, 'To' instruction.
   /// Users should provide two positions they are interested in, and the class
   /// determines (and caches) reachability.
   bool isAssumedReachable(Attributor &A, const Instruction &From,
                           const Instruction &To) const {
-    return A.getInfoCache().getPotentiallyReachable(From, To);
+    if (!A.getInfoCache().getPotentiallyReachable(From, To))
+      return false;
+    return getAssumedReachable(From, To);
   }
 
   /// Returns true if 'From' instruction is known to reach, 'To' instruction.
   /// Users should provide two positions they are interested in, and the class
   /// determines (and caches) reachability.
   bool isKnownReachable(Attributor &A, const Instruction &From,
                         const Instruction &To) const {
     return A.getInfoCache().getPotentiallyReachable(From, To);

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

return clampStateAndIndicateChange(
         getState(),
         static_cast<const AAWillReturn::StateType &>(FnAA.getState()));
   }
 
   /// See AbstractAttribute::trackStatistics()
   void trackStatistics() const override { STATS_DECLTRACK_CS_ATTR(willreturn); }
 };
 
-/// -------------------AAReachability Attribute--------------------------
-
-struct AAReachabilityImpl : AAReachability {
-  AAReachabilityImpl(const IRPosition &IRP, Attributor &A)
-      : AAReachability(IRP, A) {}
-
-  const std::string getAsStr() const override {
-    // TODO: Return the number of reachable queries.
-    return "reachable";
-  }
-
-  /// See AbstractAttribute::initialize(...).
-  void initialize(Attributor &A) override { indicatePessimisticFixpoint(); }
-
-  /// See AbstractAttribute::updateImpl(...).
-  ChangeStatus updateImpl(Attributor &A) override {
-    return indicatePessimisticFixpoint();
-  }
-};
-
-struct AAReachabilityFunction final : public AAReachabilityImpl {
-  AAReachabilityFunction(const IRPosition &IRP, Attributor &A)
-      : AAReachabilityImpl(IRP, A) {}
-
-  /// See AbstractAttribute::trackStatistics()
-  void trackStatistics() const override { STATS_DECLTRACK_FN_ATTR(reachable); }
-};
-
 /// ------------------------ NoAlias Argument Attribute ------------------------
 
 struct AANoAliasImpl : AANoAlias {
   AANoAliasImpl(const IRPosition &IRP, Attributor &A) : AANoAlias(IRP, A) {
     assert(getAssociatedType()->isPointerTy() &&
            "Noalias is a pointer attribute");
   }
 
   const std::string getAsStr() const override {
     return getAssumed() ? "noalias" : "may-alias";
   }
 };
 
 /// NoAlias attribute for a floating value.
 struct AANoAliasFloating final : AANoAliasImpl {
   AANoAliasFloating(const IRPosition &IRP, Attributor &A)
       : AANoAliasImpl(IRP, A) {}
 
   /// See AbstractAttribute::initialize(...).
else if (isa<ConstantPointerNull>(Val) &&
                                    Val->getType()->getPointerAddressSpace()))
       indicateOptimisticFixpoint();
     else if (Val != &getAssociatedValue()) {
       const auto &ValNoAliasAA =
           A.getAAFor<AANoAlias>(*this, IRPosition::value(*Val));
       if (ValNoAliasAA.isKnownNoAlias())
         indicateOptimisticFixpoint();
     }
   }
 
   /// See AbstractAttribute::updateImpl(...).
   ChangeStatus updateImpl(Attributor &A) override {
     // TODO: Implement this.
     return indicatePessimisticFixpoint();
   }
 
   /// See AbstractAttribute::trackStatistics()
struct AAIsDeadCallSite final : AAIsDeadFunction {
   ChangeStatus updateImpl(Attributor &A) override {
     return indicatePessimisticFixpoint();
   }
 
   /// See AbstractAttribute::trackStatistics()
   void trackStatistics() const override {}
 };
 
+/// -------------------AAReachability Attribute--------------------------
+
+struct AAReachabilityImpl : AAReachability {
+  AAReachabilityImpl(const IRPosition &IRP, Attributor &A)
+      : AAReachability(IRP, A) {}
+
+  const std::string getAsStr() const override {
+    // TODO: Return the number of reachable queries.
+    return "reachable";
+  }
+
+  /// See AbstractAttribute::initialize(...).
+  void initialize(Attributor &A) override { indicatePessimisticFixpoint(); }
+
+  /// See AbstractAttribute::updateImpl(...).
+  ChangeStatus updateImpl(Attributor &A) override {
+    return indicatePessimisticFixpoint();
+  }
+};
+
+struct AAReachabilityFunction final : public AAReachabilityImpl {
+  AAReachabilityFunction(const IRPosition &IRP, Attributor &A)
+      : AAReachabilityImpl(IRP, A) {}
+  /// See AbstractAttribute::initialize(...).
+  void initialize(Attributor &A) override {}
+
+  /// Helper function for updateImpl to add \p ToBB as successor of \p FromBB
+  ChangeStatus addEdge(const BasicBlock *FromBB, const BasicBlock *ToBB) {
+    auto InsertResult = SuccessorGraph.insert(
+        std::make_pair(FromBB, SetVector<const BasicBlock *>()));
+    auto SetInsertResult = InsertResult.first->second.insert(ToBB);
+    if (SetInsertResult == true) {
+      LLVM_DEBUG(dbgs() << "[AAReachability] Add edge : " << *FromBB << " -> "
+                        << *ToBB << "\n");
+    }
+    return SetInsertResult ? ChangeStatus::CHANGED : ChangeStatus::UNCHANGED;
+  }
+
+  /// See AbstractAttribute::updateImpl(...).
+  ChangeStatus updateImpl(Attributor &A) override {
+    ChangeStatus Changed = ChangeStatus::UNCHANGED;
+    Function &F = *getAnchorScope();
+    const auto &FnLiveness = A.getAAFor<AAIsDead>(*this, getIRPosition());
+    for (Function::const_iterator BBIt = F.begin(); BBIt != F.end(); BBIt++) {
+      if (A.isAssumedDead(BBIt->front(), this, &FnLiveness,
+                          /* CheckBBLivenessOnly */ true, DepClassTy::REQUIRED))
+        continue;
+      SmallVector<const Instruction *, 8> AliveSuccessors;
+      for (BasicBlock::const_iterator InstIt = BBIt->begin();
+           InstIt != BBIt->end(); InstIt++) {
+        const Instruction *I = &*InstIt;
+        if (A.isAssumedDead(*I, this, &FnLiveness))
+          continue;
+        switch (I->getOpcode()) {
+        default:
+          if (I->isTerminator()) {
+            for (const BasicBlock *SuccBB : successors(I->getParent()))
+              AliveSuccessors.push_back(&SuccBB->front());
+          }
+          break;
+        case Instruction::Call:
+          identifyAliveSuccessors(A, cast<CallInst>(*I), *this,
+                                  AliveSuccessors);
+          break;
+        case Instruction::Invoke:
+          identifyAliveSuccessors(A, cast<InvokeInst>(*I), *this,
+                                  AliveSuccessors);
+          break;
+        case Instruction::Br:
+          identifyAliveSuccessors(A, cast<BranchInst>(*I), *this,
+                                  AliveSuccessors);
+          break;
+        case Instruction::Switch:
+          identifyAliveSuccessors(A, cast<SwitchInst>(*I), *this,
+                                  AliveSuccessors);
+          break;
+        }
+      }
+      const BasicBlock *FromBB = &*BBIt;
+      for (const Instruction *I : AliveSuccessors) {
+        if (I->getParent() == FromBB)
+          continue;
+        Changed = Changed | addEdge(FromBB, I->getParent());
+      }
+    }
+    return Changed;
+  }
+
+  /// Return wheather \p To is reachable from \p From or not.
+  bool getAssumedReachable(const Instruction &From,
+                           const Instruction &To) const override {
+    const BasicBlock *FromBB = From.getParent();
+    const BasicBlock *ToBB = To.getParent();
+
+    if (FromBB == ToBB)
+      return true;
+
+    /// Basic blocks which are confirmed to be rechable from \p FromBB
+    SetVector<const BasicBlock *> ReachableBBs;
+
+    SmallVector<const BasicBlock *, 8> Worklist;
+    Worklist.push_back(FromBB);
+
+    while (!Worklist.empty()) {
+      const BasicBlock *BB = Worklist.pop_back_val();
+      LLVM_DEBUG(dbgs() << "[AAReachability] Explore BB : " << *BB << "\n");
+      if (BB == ToBB)
+        return true;
+      auto GraphIt = SuccessorGraph.find(BB);
+      if (GraphIt == SuccessorGraph.end())
+        continue;
+      for (const BasicBlock *NextBB : GraphIt->second) {
+        if (!ReachableBBs.count(NextBB)) {
+          Worklist.push_back(NextBB);
+          ReachableBBs.insert(NextBB);
+        }
+      }
+    }
+    return false;
+  }
+
+  /// See AbstractAttribute::trackStatistics()
+  void trackStatistics() const override { STATS_DECLTRACK_FN_ATTR(reachable); }
+
+  /// Map from a basic block to its reachable successor basic blocks
+  DenseMap<const BasicBlock *, SetVector<const BasicBlock *>> SuccessorGraph;
+};
+
 /// -------------------- Dereferenceable Argument Attribute --------------------
 
 template <>
 ChangeStatus clampStateAndIndicateChange<DerefState>(DerefState &S,
                                                      const DerefState &R) {
   ChangeStatus CS0 =
       clampStateAndIndicateChange(S.DerefBytesState, R.DerefBytesState);
   ChangeStatus CS1 = clampStateAndIndicateChange(S.GlobalState, R.GlobalState);

llvm/test/Transforms/Attributor/callbacks.ll

 
 define void @t0_caller(i32* %a) {
 ; IS__TUNIT_OPM-LABEL: define {{[^@]+}}@t0_caller
 ; IS__TUNIT_OPM-SAME: (i32* align 256 [[A:%.*]])
 ; IS__TUNIT_OPM-NEXT:  entry:
 ; IS__TUNIT_OPM-NEXT:    [[B:%.*]] = alloca i32, align 32
 ; IS__TUNIT_OPM-NEXT:    [[C:%.*]] = alloca i32*, align 64
 ; IS__TUNIT_OPM-NEXT:    [[PTR:%.*]] = alloca i32, align 128
-; IS__TUNIT_OPM-NEXT:    [[TMP0:%.*]] = bitcast i32* [[B]] to i8*
 ; IS__TUNIT_OPM-NEXT:    store i32 42, i32* [[B]], align 32
 ; IS__TUNIT_OPM-NEXT:    store i32* [[B]], i32** [[C]], align 64
 ; IS__TUNIT_OPM-NEXT:    call void (i32*, i32*, void (i32*, i32*, ...)*, ...) @t0_callback_broker(i32* noalias nocapture align 536870912 null, i32* nonnull align 128 dereferenceable(4) [[PTR]], void (i32*, i32*, ...)* nonnull bitcast (void (i32*, i32*, i32*, i64, i32**)* @t0_callback_callee to void (i32*, i32*, ...)*), i32* align 256 [[A]], i64 undef, i32** nocapture nonnull readonly align 64 dereferenceable(8) [[C]])
 ; IS__TUNIT_OPM-NEXT:    ret void
 ;
 ; IS__TUNIT_NPM-LABEL: define {{[^@]+}}@t0_caller
 ; IS__TUNIT_NPM-SAME: (i32* align 256 [[A:%.*]])
 ; IS__TUNIT_NPM-NEXT:  entry:
 ; IS__TUNIT_NPM-NEXT:    [[B:%.*]] = alloca i32, align 32
 ; IS__TUNIT_NPM-NEXT:    [[C:%.*]] = alloca i32*, align 64
 ; IS__TUNIT_NPM-NEXT:    [[PTR:%.*]] = alloca i32, align 128
-; IS__TUNIT_NPM-NEXT:    [[TMP0:%.*]] = bitcast i32* [[B]] to i8*
 ; IS__TUNIT_NPM-NEXT:    store i32 42, i32* [[B]], align 32
 ; IS__TUNIT_NPM-NEXT:    store i32* [[B]], i32** [[C]], align 64
 ; IS__TUNIT_NPM-NEXT:    call void (i32*, i32*, void (i32*, i32*, ...)*, ...) @t0_callback_broker(i32* noalias nocapture align 536870912 null, i32* nonnull align 128 dereferenceable(4) [[PTR]], void (i32*, i32*, ...)* nonnull bitcast (void (i32*, i32*, i32*, i64, i32**)* @t0_callback_callee to void (i32*, i32*, ...)*), i32* align 256 [[A]], i64 undef, i32** noalias nocapture nonnull readonly align 64 dereferenceable(8) [[C]])
 ; IS__TUNIT_NPM-NEXT:    ret void
 ;
 ; IS__CGSCC_OPM-LABEL: define {{[^@]+}}@t0_caller
 ; IS__CGSCC_OPM-SAME: (i32* align 256 [[A:%.*]])
 ; IS__CGSCC_OPM-NEXT:  entry:
 ; IS__CGSCC_OPM-NEXT:    [[B:%.*]] = alloca i32, align 32
 ; IS__CGSCC_OPM-NEXT:    [[C:%.*]] = alloca i32*, align 64
 ; IS__CGSCC_OPM-NEXT:    [[PTR:%.*]] = alloca i32, align 128
-; IS__CGSCC_OPM-NEXT:    [[TMP0:%.*]] = bitcast i32* [[B]] to i8*
 ; IS__CGSCC_OPM-NEXT:    store i32 42, i32* [[B]], align 32
 ; IS__CGSCC_OPM-NEXT:    store i32* [[B]], i32** [[C]], align 64
 ; IS__CGSCC_OPM-NEXT:    call void (i32*, i32*, void (i32*, i32*, ...)*, ...) @t0_callback_broker(i32* noalias nocapture align 536870912 null, i32* nonnull align 128 dereferenceable(4) [[PTR]], void (i32*, i32*, ...)* bitcast (void (i32*, i32*, i32*, i64, i32**)* @t0_callback_callee to void (i32*, i32*, ...)*), i32* align 256 [[A]], i64 99, i32** nocapture nonnull readonly align 64 dereferenceable(8) [[C]])
 ; IS__CGSCC_OPM-NEXT:    ret void
 ;
 ; IS__CGSCC_NPM-LABEL: define {{[^@]+}}@t0_caller
 ; IS__CGSCC_NPM-SAME: (i32* align 256 [[A:%.*]])
 ; IS__CGSCC_NPM-NEXT:  entry:
 ; IS__CGSCC_NPM-NEXT:    [[B:%.*]] = alloca i32, align 32
 ; IS__CGSCC_NPM-NEXT:    [[C:%.*]] = alloca i32*, align 64
 ; IS__CGSCC_NPM-NEXT:    [[PTR:%.*]] = alloca i32, align 128
-; IS__CGSCC_NPM-NEXT:    [[TMP0:%.*]] = bitcast i32* [[B]] to i8*
 ; IS__CGSCC_NPM-NEXT:    store i32 42, i32* [[B]], align 32
 ; IS__CGSCC_NPM-NEXT:    store i32* [[B]], i32** [[C]], align 64
 ; IS__CGSCC_NPM-NEXT:    call void (i32*, i32*, void (i32*, i32*, ...)*, ...) @t0_callback_broker(i32* noalias nocapture align 536870912 null, i32* nonnull align 128 dereferenceable(4) [[PTR]], void (i32*, i32*, ...)* bitcast (void (i32*, i32*, i32*, i64, i32**)* @t0_callback_callee to void (i32*, i32*, ...)*), i32* align 256 [[A]], i64 99, i32** noalias nocapture nonnull readonly align 64 dereferenceable(8) [[C]])
 ; IS__CGSCC_NPM-NEXT:    ret void
 ;
 entry:
   %b = alloca i32, align 32
   %c = alloca i32*, align 64

llvm/test/Transforms/Attributor/noalias.ll

 ; NOT_CGSCC_NPM-SAME: (i32* noalias nofree writeonly [[P:%.*]], i32 [[C:%.*]])
 ; NOT_CGSCC_NPM-NEXT:  entry:
 ; NOT_CGSCC_NPM-NEXT:    [[TOBOOL:%.*]] = icmp eq i32 [[C]], 0
 ; NOT_CGSCC_NPM-NEXT:    br i1 [[TOBOOL]], label [[L1:%.*]], label [[L2:%.*]]
 ; NOT_CGSCC_NPM:       l1:
 ; NOT_CGSCC_NPM-NEXT:    tail call void @make_alias(i32* nofree writeonly [[P]])
 ; NOT_CGSCC_NPM-NEXT:    br label [[L3:%.*]]
 ; NOT_CGSCC_NPM:       l2:
-; NOT_CGSCC_NPM-NEXT:    tail call void @only_store(i32* nocapture nofree writeonly align 4 [[P]])
+; NOT_CGSCC_NPM-NEXT:    tail call void @only_store(i32* noalias nocapture nofree writeonly align 4 [[P]])
 ; NOT_CGSCC_NPM-NEXT:    br label [[L3]]
 ; NOT_CGSCC_NPM:       l3:
 ; NOT_CGSCC_NPM-NEXT:    ret void
 ;
 ; IS__CGSCC____: Function Attrs: nofree norecurse nosync nounwind willreturn writeonly
 ; IS__CGSCC____-LABEL: define {{[^@]+}}@test17_caller
 ; IS__CGSCC____-SAME: (i32* noalias nofree writeonly [[P:%.*]], i32 [[C:%.*]])
 ; IS__CGSCC____-NEXT:  entry:
 ; IS__CGSCC____-NEXT:    [[TOBOOL:%.*]] = icmp eq i32 [[C]], 0
 ; IS__CGSCC____-NEXT:    br i1 [[TOBOOL]], label [[L1:%.*]], label [[L2:%.*]]
 ; IS__CGSCC____:       l1:
 ; IS__CGSCC____-NEXT:    tail call void @make_alias(i32* nofree writeonly [[P]])
 ; IS__CGSCC____-NEXT:    br label [[L3:%.*]]
 ; IS__CGSCC____:       l2:
-; IS__CGSCC____-NEXT:    tail call void @only_store(i32* nocapture nofree nonnull writeonly align 4 dereferenceable(4) [[P]])
+; IS__CGSCC____-NEXT:    tail call void @only_store(i32* noalias nocapture nofree nonnull writeonly align 4 dereferenceable(4) [[P]])
 ; IS__CGSCC____-NEXT:    br label [[L3]]
 ; IS__CGSCC____:       l3:
 ; IS__CGSCC____-NEXT:    ret void
 ;
 entry:
   %tobool = icmp eq i32 %c, 0
   br i1 %tobool, label %l1, label %l2
 

 ; NOT_CGSCC_NPM-SAME: (i32* noalias nofree writeonly [[P:%.*]], i32 [[C:%.*]])
 ; NOT_CGSCC_NPM-NEXT:  entry:
 ; NOT_CGSCC_NPM-NEXT:    [[TOBOOL:%.*]] = icmp eq i32 [[C]], 0
 ; NOT_CGSCC_NPM-NEXT:    br i1 [[TOBOOL]], label [[L1:%.*]], label [[L2:%.*]]
 ; NOT_CGSCC_NPM:       l1:
 ; NOT_CGSCC_NPM-NEXT:    tail call void @make_alias(i32* nofree writeonly [[P]])
 ; NOT_CGSCC_NPM-NEXT:    unreachable
 ; NOT_CGSCC_NPM:       l2:
-; NOT_CGSCC_NPM-NEXT:    tail call void @only_store(i32* nocapture nofree writeonly align 4 [[P]])
+; NOT_CGSCC_NPM-NEXT:    tail call void @only_store(i32* noalias nocapture nofree writeonly align 4 [[P]])
 ; NOT_CGSCC_NPM-NEXT:    ret void
 ;
 ; IS__CGSCC____: Function Attrs: nofree norecurse nosync nounwind willreturn writeonly
 ; IS__CGSCC____-LABEL: define {{[^@]+}}@test18_caller
 ; IS__CGSCC____-SAME: (i32* noalias nofree nonnull writeonly align 4 dereferenceable(4) [[P:%.*]], i32 [[C:%.*]])
 ; IS__CGSCC____-NEXT:  entry:
 ; IS__CGSCC____-NEXT:    [[TOBOOL:%.*]] = icmp eq i32 [[C]], 0
 ; IS__CGSCC____-NEXT:    br i1 [[TOBOOL]], label [[L1:%.*]], label [[L2:%.*]]
 ; IS__CGSCC____:       l1:
 ; IS__CGSCC____-NEXT:    tail call void @make_alias(i32* nofree nonnull writeonly align 4 dereferenceable(4) [[P]])
 ; IS__CGSCC____-NEXT:    unreachable
 ; IS__CGSCC____:       l2:
-; IS__CGSCC____-NEXT:    tail call void @only_store(i32* nocapture nofree nonnull writeonly align 4 dereferenceable(4) [[P]])
+; IS__CGSCC____-NEXT:    tail call void @only_store(i32* noalias nocapture nofree nonnull writeonly align 4 dereferenceable(4) [[P]])
 ; IS__CGSCC____-NEXT:    ret void
 ;
 entry:
   %tobool = icmp eq i32 %c, 0
   br i1 %tobool, label %l1, label %l2
 
 l1:
   tail call void @make_alias(i32* %p)
   tail call void @noreturn()
   br label %l2
 
 l2:
   tail call void @only_store(i32* %p)
   ret void
 }

llvm/test/Transforms/Attributor/read_write_returned_arguments_scc.ll

 ; IS__TUNIT____-NEXT:    [[CALL3:%.*]] = call i32* @internal_ret0_nw(i32* nofree nonnull align 4 dereferenceable(4) [[W0]], i32* nofree nonnull align 4 dereferenceable(4) [[W0]])
 ; IS__TUNIT____-NEXT:    [[CALL4:%.*]] = call i32* @external_ret2_nrw(i32* nofree nonnull align 4 dereferenceable(4) [[R0]], i32* nofree nonnull align 4 dereferenceable(4) [[R1]], i32* nofree nonnull align 4 dereferenceable(4) [[W0]])
 ; IS__TUNIT____-NEXT:    [[CALL5:%.*]] = call i32* @external_ret2_nrw(i32* nofree nonnull align 4 dereferenceable(4) [[R1]], i32* nofree nonnull align 4 dereferenceable(4) [[R0]], i32* nofree nonnull align 4 dereferenceable(4) [[W0]])
 ; IS__TUNIT____-NEXT:    [[CALL6:%.*]] = call i32* @external_sink_ret2_nrw(i32* nofree nonnull align 4 dereferenceable(4) [[R0]], i32* nocapture nofree nonnull readonly align 4 dereferenceable(4) [[R1]], i32* nofree nonnull writeonly align 4 dereferenceable(4) "no-capture-maybe-returned" [[W0]])
 ; IS__TUNIT____-NEXT:    [[CALL7:%.*]] = call i32* @external_sink_ret2_nrw(i32* nofree nonnull align 4 dereferenceable(4) [[R1]], i32* nocapture nofree nonnull readonly align 4 dereferenceable(4) [[R0]], i32* nofree nonnull writeonly align 4 dereferenceable(4) "no-capture-maybe-returned" [[W0]])
 ; IS__TUNIT____-NEXT:    [[CALL8:%.*]] = call i32* @internal_ret0_nw(i32* nofree nonnull align 4 dereferenceable(4) [[R1]], i32* nofree nonnull align 4 dereferenceable(4) [[W0]])
 ; IS__TUNIT____-NEXT:    br label [[RETURN]]
 ; IS__TUNIT____:       return:
-; IS__TUNIT____-NEXT:    [[RETVAL_0:%.*]] = phi i32* [ [[CALL8]], [[IF_END]] ], [ [[R1]], [[IF_THEN]] ]
 ; IS__TUNIT____-NEXT:    ret i32* undef
 ;
 ; IS__CGSCC____: Function Attrs: argmemonly nofree nosync nounwind
 ; IS__CGSCC____-LABEL: define {{[^@]+}}@internal_ret1_rrw
 ; IS__CGSCC____-SAME: (i32* nofree nonnull align 4 dereferenceable(4) [[R0:%.*]], i32* nofree returned [[R1:%.*]], i32* nofree [[W0:%.*]])
 ; IS__CGSCC____-NEXT:  entry:
 ; IS__CGSCC____-NEXT:    [[TMP0:%.*]] = load i32, i32* [[R0]], align 4
 ; IS__CGSCC____-NEXT:    [[TOBOOL:%.*]] = icmp ne i32 [[TMP0]], 0

 ; IS__CGSCC____-NEXT:    [[CALL3:%.*]] = call i32* @internal_ret0_nw(i32* nofree nonnull align 4 dereferenceable(4) [[W0]], i32* nofree nonnull align 4 dereferenceable(4) [[W0]])
 ; IS__CGSCC____-NEXT:    [[CALL4:%.*]] = call i32* @external_ret2_nrw(i32* nofree nonnull align 4 dereferenceable(4) [[R0]], i32* nofree nonnull align 4 dereferenceable(4) [[R1]], i32* nofree nonnull align 4 dereferenceable(4) [[W0]])
 ; IS__CGSCC____-NEXT:    [[CALL5:%.*]] = call i32* @external_ret2_nrw(i32* nofree nonnull align 4 dereferenceable(4) [[R1]], i32* nofree nonnull align 4 dereferenceable(4) [[R0]], i32* nofree nonnull align 4 dereferenceable(4) [[W0]])
 ; IS__CGSCC____-NEXT:    [[CALL6:%.*]] = call i32* @external_sink_ret2_nrw(i32* nofree nonnull align 4 dereferenceable(4) [[R0]], i32* nocapture nofree nonnull readonly align 4 dereferenceable(4) [[R1]], i32* nofree nonnull writeonly align 4 dereferenceable(4) [[W0]])
 ; IS__CGSCC____-NEXT:    [[CALL7:%.*]] = call i32* @external_sink_ret2_nrw(i32* nofree nonnull align 4 dereferenceable(4) [[R1]], i32* nocapture nofree nonnull readonly align 4 dereferenceable(4) [[R0]], i32* nofree nonnull writeonly align 4 dereferenceable(4) [[W0]])
 ; IS__CGSCC____-NEXT:    [[CALL8:%.*]] = call i32* @internal_ret0_nw(i32* nofree nonnull align 4 dereferenceable(4) [[R1]], i32* nofree nonnull align 4 dereferenceable(4) [[W0]])
 ; IS__CGSCC____-NEXT:    br label [[RETURN]]
 ; IS__CGSCC____:       return:
-; IS__CGSCC____-NEXT:    [[RETVAL_0:%.*]] = phi i32* [ [[CALL8]], [[IF_END]] ], [ [[R1]], [[IF_THEN]] ]
 ; IS__CGSCC____-NEXT:    ret i32* undef
 ;
 entry:
   %0 = load i32, i32* %r0, align 4
   %tobool = icmp ne i32 %0, 0
   br i1 %tobool, label %if.end, label %if.then
 
 if.then:                                          ; preds = %entry