[FunctionAttrs] Remove old "no-capture" and memory behavior argument deduction

[jdoerfert]

Summary

This removes the old FunctionAttrs "no-capture" and memory behavior
argument deduction.

The code is obsolete as the Attributor deduction is generally more
powerful.

The test cases that were affected by the change now run both passes.

This resolves incorrect behavior (see http://llvm.org/PR41328) as shown
by the changes to "test13" in:

`test/Transforms/FunctionAttrs/readattrs.ll`

Impact on the statistics (-stats) for LLVM-TS + Spec2006:

REMOVED: functionattrs                NumNoCapture                              17 ->        n/a
REMOVED: functionattrs                NumReadNoneArg                           145 ->        n/a
REMOVED: functionattrs                NumReadOnlyArg                           946 ->        n/a

Note that "no-capture" was derived by the now removed code for arguments
that were already annotated. The readnone/only attributes are at least
in parts due to aforementioned bug.

[reames]

Please post for review once attributor is actually enabled by default. Removing existing code before that point is inappropriate.

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

 
 using namespace llvm;
 
 #define DEBUG_TYPE "functionattrs"
 
 STATISTIC(NumReadNone, "Number of functions marked readnone");
 STATISTIC(NumReadOnly, "Number of functions marked readonly");
 STATISTIC(NumWriteOnly, "Number of functions marked writeonly");
-STATISTIC(NumNoCapture, "Number of arguments marked nocapture");
-STATISTIC(NumReadNoneArg, "Number of arguments marked readnone");
-STATISTIC(NumReadOnlyArg, "Number of arguments marked readonly");
 STATISTIC(NumNoAlias, "Number of function returns marked noalias");
 STATISTIC(NumNonNullReturn, "Number of function returns marked nonnull");
 STATISTIC(NumNoRecurse, "Number of functions marked as norecurse");
 STATISTIC(NumNoUnwind, "Number of functions marked as nounwind");
 
 // FIXME: This is disabled by default to avoid exposing security vulnerabilities
 // in C/C++ code compiled by clang:
 // http://lists.llvm.org/pipermail/cfe-dev/2017-January/052066.html
else if (ReadsMemory)
       ++NumReadOnly;
     else
       ++NumReadNone;
   }
 
   return MadeChange;
 }
 
-namespace {
-
-/// For a given pointer Argument, this retains a list of Arguments of functions
-/// in the same SCC that the pointer data flows into. We use this to build an
-/// SCC of the arguments.
-struct ArgumentGraphNode {
-  Argument *Definition;
-  SmallVector<ArgumentGraphNode *, 4> Uses;
-};
-
-class ArgumentGraph {
-  // We store pointers to ArgumentGraphNode objects, so it's important that
-  // that they not move around upon insert.
-  using ArgumentMapTy = std::map<Argument *, ArgumentGraphNode>;
-
-  ArgumentMapTy ArgumentMap;
-
-  // There is no root node for the argument graph, in fact:
-  //   void f(int *x, int *y) { if (...) f(x, y); }
-  // is an example where the graph is disconnected. The SCCIterator requires a
-  // single entry point, so we maintain a fake ("synthetic") root node that
-  // uses every node. Because the graph is directed and nothing points into
-  // the root, it will not participate in any SCCs (except for its own).
-  ArgumentGraphNode SyntheticRoot;
-
-public:
-  ArgumentGraph() { SyntheticRoot.Definition = nullptr; }
-
-  using iterator = SmallVectorImpl<ArgumentGraphNode *>::iterator;
-
-  iterator begin() { return SyntheticRoot.Uses.begin(); }
-  iterator end() { return SyntheticRoot.Uses.end(); }
-  ArgumentGraphNode *getEntryNode() { return &SyntheticRoot; }
-
-  ArgumentGraphNode *operator[](Argument *A) {
-    ArgumentGraphNode &Node = ArgumentMap[A];
-    Node.Definition = A;
-    SyntheticRoot.Uses.push_back(&Node);
-    return &Node;
-  }
-};
-
-/// This tracker checks whether callees are in the SCC, and if so it does not
-/// consider that a capture, instead adding it to the "Uses" list and
-/// continuing with the analysis.
-struct ArgumentUsesTracker : public CaptureTracker {
-  ArgumentUsesTracker(const SCCNodeSet &SCCNodes) : SCCNodes(SCCNodes) {}
-
-  void tooManyUses() override { Captured = true; }
-
-  bool captured(const Use *U) override {
-    CallSite CS(U->getUser());
-    if (!CS.getInstruction()) {
-      Captured = true;
-      return true;
-    }
-
-    Function *F = CS.getCalledFunction();
-    if (!F || !F->hasExactDefinition() || !SCCNodes.count(F)) {
-      Captured = true;
-      return true;
-    }
-
-    // Note: the callee and the two successor blocks *follow* the argument
-    // operands.  This means there is no need to adjust UseIndex to account for
-    // these.
-
-    unsigned UseIndex =
-        std::distance(const_cast<const Use *>(CS.arg_begin()), U);
-
-    assert(UseIndex < CS.data_operands_size() &&
-           "Indirect function calls should have been filtered above!");
-
-    if (UseIndex >= CS.getNumArgOperands()) {
-      // Data operand, but not a argument operand -- must be a bundle operand
-      assert(CS.hasOperandBundles() && "Must be!");
-
-      // CaptureTracking told us that we're being captured by an operand bundle
-      // use.  In this case it does not matter if the callee is within our SCC
-      // or not -- we've been captured in some unknown way, and we have to be
-      // conservative.
-      Captured = true;
-      return true;
-    }
-
-    if (UseIndex >= F->arg_size()) {
-      assert(F->isVarArg() && "More params than args in non-varargs call");
-      Captured = true;
-      return true;
-    }
-
-    Uses.push_back(&*std::next(F->arg_begin(), UseIndex));
-    return false;
-  }
-
-  // True only if certainly captured (used outside our SCC).
-  bool Captured = false;
-
-  // Uses within our SCC.
-  SmallVector<Argument *, 4> Uses;
-
-  const SCCNodeSet &SCCNodes;
-};
-
-} // end anonymous namespace
-
-namespace llvm {
-
-template <> struct GraphTraits<ArgumentGraphNode *> {
-  using NodeRef = ArgumentGraphNode *;
-  using ChildIteratorType = SmallVectorImpl<ArgumentGraphNode *>::iterator;
-
-  static NodeRef getEntryNode(NodeRef A) { return A; }
-  static ChildIteratorType child_begin(NodeRef N) { return N->Uses.begin(); }
-  static ChildIteratorType child_end(NodeRef N) { return N->Uses.end(); }
-};
-
-template <>
-struct GraphTraits<ArgumentGraph *> : public GraphTraits<ArgumentGraphNode *> {
-  static NodeRef getEntryNode(ArgumentGraph *AG) { return AG->getEntryNode(); }
-
-  static ChildIteratorType nodes_begin(ArgumentGraph *AG) {
-    return AG->begin();
-  }
-
-  static ChildIteratorType nodes_end(ArgumentGraph *AG) { return AG->end(); }
-};
-
-} // end namespace llvm
-
-/// Returns Attribute::None, Attribute::ReadOnly or Attribute::ReadNone.
-static Attribute::AttrKind
-determinePointerReadAttrs(Argument *A,
-                          const SmallPtrSet<Argument *, 8> &SCCNodes) {
-  SmallVector<Use *, 32> Worklist;
-  SmallPtrSet<Use *, 32> Visited;
-
-  // inalloca arguments are always clobbered by the call.
-  if (A->hasInAllocaAttr())
-    return Attribute::None;
-
-  bool IsRead = false;
-  // We don't need to track IsWritten. If A is written to, return immediately.
-
-  for (Use &U : A->uses()) {
-    Visited.insert(&U);
-    Worklist.push_back(&U);
-  }
-
-  while (!Worklist.empty()) {
-    Use *U = Worklist.pop_back_val();
-    Instruction *I = cast<Instruction>(U->getUser());
-
-    switch (I->getOpcode()) {
-    case Instruction::BitCast:
-    case Instruction::GetElementPtr:
-    case Instruction::PHI:
-    case Instruction::Select:
-    case Instruction::AddrSpaceCast:
-      // The original value is not read/written via this if the new value isn't.
-      for (Use &UU : I->uses())
-        if (Visited.insert(&UU).second)
-          Worklist.push_back(&UU);
-      break;
-
-    case Instruction::Call:
-    case Instruction::Invoke: {
-      bool Captures = true;
-
-      if (I->getType()->isVoidTy())
-        Captures = false;
-
-      auto AddUsersToWorklistIfCapturing = [&] {
-        if (Captures)
-          for (Use &UU : I->uses())
-            if (Visited.insert(&UU).second)
-              Worklist.push_back(&UU);
-      };
-
-      CallSite CS(I);
-      if (CS.doesNotAccessMemory()) {
-        AddUsersToWorklistIfCapturing();
-        continue;
-      }
-
-      Function *F = CS.getCalledFunction();
-      if (!F) {
-        if (CS.onlyReadsMemory()) {
-          IsRead = true;
-          AddUsersToWorklistIfCapturing();
-          continue;
-        }
-        return Attribute::None;
-      }
-
-      // Note: the callee and the two successor blocks *follow* the argument
-      // operands.  This means there is no need to adjust UseIndex to account
-      // for these.
-
-      unsigned UseIndex = std::distance(CS.arg_begin(), U);
-
-      // U cannot be the callee operand use: since we're exploring the
-      // transitive uses of an Argument, having such a use be a callee would
-      // imply the CallSite is an indirect call or invoke; and we'd take the
-      // early exit above.
-      assert(UseIndex < CS.data_operands_size() &&
-             "Data operand use expected!");
-
-      bool IsOperandBundleUse = UseIndex >= CS.getNumArgOperands();
-
-      if (UseIndex >= F->arg_size() && !IsOperandBundleUse) {
-        assert(F->isVarArg() && "More params than args in non-varargs call");
-        return Attribute::None;
-      }
-
-      Captures &= !CS.doesNotCapture(UseIndex);
-
-      // Since the optimizer (by design) cannot see the data flow corresponding
-      // to a operand bundle use, these cannot participate in the optimistic SCC
-      // analysis.  Instead, we model the operand bundle uses as arguments in
-      // call to a function external to the SCC.
-      if (IsOperandBundleUse ||
-          !SCCNodes.count(&*std::next(F->arg_begin(), UseIndex))) {
-
-        // The accessors used on CallSite here do the right thing for calls and
-        // invokes with operand bundles.
-
-        if (!CS.onlyReadsMemory() && !CS.onlyReadsMemory(UseIndex))
-          return Attribute::None;
-        if (!CS.doesNotAccessMemory(UseIndex))
-          IsRead = true;
-      }
-
-      AddUsersToWorklistIfCapturing();
-      break;
-    }
-
-    case Instruction::Load:
-      // A volatile load has side effects beyond what readonly can be relied
-      // upon.
-      if (cast<LoadInst>(I)->isVolatile())
-        return Attribute::None;
-
-      IsRead = true;
-      break;
-
-    case Instruction::ICmp:
-    case Instruction::Ret:
-      break;
-
-    default:
-      return Attribute::None;
-    }
-  }
-
-  return IsRead ? Attribute::ReadOnly : Attribute::ReadNone;
-}
-
 /// If a callsite has arguments that are also arguments to the parent function,
 /// try to propagate attributes from the callsite's arguments to the parent's
 /// arguments. This may be important because inlining can cause information loss
 /// when attribute knowledge disappears with the inlined call.
 static bool addArgumentAttrsFromCallsites(Function &F) {
   if (!EnableNonnullArgPropagation)
     return false;
 
for (Instruction &I : Entry) {
     }
     if (!isGuaranteedToTransferExecutionToSuccessor(&I))
       break;
   }
 
   return Changed;
 }
 
-/// Deduce nocapture attributes for the SCC.
+/// Deduce nonull attributes for the SCC.
 static bool addArgumentAttrs(const SCCNodeSet &SCCNodes) {
   bool Changed = false;
 
-  ArgumentGraph AG;
-
   // Check each function in turn, determining which pointer arguments are not
   // captured.
   for (Function *F : SCCNodes) {
     // We can infer and propagate function attributes only when we know that the
     // definition we'll get at link time is *exactly* the definition we see now.
     // For more details, see GlobalValue::mayBeDerefined.
     if (!F->hasExactDefinition())
       continue;
 
     Changed |= addArgumentAttrsFromCallsites(*F);
 
-    // Functions that are readonly (or readnone) and nounwind and don't return
-    // a value can't capture arguments. Don't analyze them.
-    if (F->onlyReadsMemory() && F->doesNotThrow() &&
-        F->getReturnType()->isVoidTy()) {
-      for (Function::arg_iterator A = F->arg_begin(), E = F->arg_end(); A != E;
-           ++A) {
-        if (A->getType()->isPointerTy() && !A->hasNoCaptureAttr()) {
-          A->addAttr(Attribute::NoCapture);
-          ++NumNoCapture;
-          Changed = true;
-        }
-      }
-      continue;
-    }
-
-    for (Function::arg_iterator A = F->arg_begin(), E = F->arg_end(); A != E;
-         ++A) {
-      if (!A->getType()->isPointerTy())
-        continue;
-      bool HasNonLocalUses = false;
-      if (!A->hasNoCaptureAttr()) {
-        ArgumentUsesTracker Tracker(SCCNodes);
-        PointerMayBeCaptured(&*A, &Tracker);
-        if (!Tracker.Captured) {
-          if (Tracker.Uses.empty()) {
-            // If it's trivially not captured, mark it nocapture now.
-            A->addAttr(Attribute::NoCapture);
-            ++NumNoCapture;
-            Changed = true;
-          } else {
-            // If it's not trivially captured and not trivially not captured,
-            // then it must be calling into another function in our SCC. Save
-            // its particulars for Argument-SCC analysis later.
-            ArgumentGraphNode *Node = AG[&*A];
-            for (Argument *Use : Tracker.Uses) {
-              Node->Uses.push_back(AG[Use]);
-              if (Use != &*A)
-                HasNonLocalUses = true;
-            }
-          }
-        }
-        // Otherwise, it's captured. Don't bother doing SCC analysis on it.
-      }
-      if (!HasNonLocalUses && !A->onlyReadsMemory()) {
-        // Can we determine that it's readonly/readnone without doing an SCC?
-        // Note that we don't allow any calls at all here, or else our result
-        // will be dependent on the iteration order through the functions in the
-        // SCC.
-        SmallPtrSet<Argument *, 8> Self;
-        Self.insert(&*A);
-        Attribute::AttrKind R = determinePointerReadAttrs(&*A, Self);
-        if (R != Attribute::None) {
-          A->addAttr(R);
-          Changed = true;
-          R == Attribute::ReadOnly ? ++NumReadOnlyArg : ++NumReadNoneArg;
-        }
-      }
-    }
-  }
-
-  // The graph we've collected is partial because we stopped scanning for
-  // argument uses once we solved the argument trivially. These partial nodes
-  // show up as ArgumentGraphNode objects with an empty Uses list, and for
-  // these nodes the final decision about whether they capture has already been
-  // made.  If the definition doesn't have a 'nocapture' attribute by now, it
-  // captures.
-
-  for (scc_iterator<ArgumentGraph *> I = scc_begin(&AG); !I.isAtEnd(); ++I) {
-    const std::vector<ArgumentGraphNode *> &ArgumentSCC = *I;
-    if (ArgumentSCC.size() == 1) {
-      if (!ArgumentSCC[0]->Definition)
-        continue; // synthetic root node
-
-      // eg. "void f(int* x) { if (...) f(x); }"
-      if (ArgumentSCC[0]->Uses.size() == 1 &&
-          ArgumentSCC[0]->Uses[0] == ArgumentSCC[0]) {
-        Argument *A = ArgumentSCC[0]->Definition;
-        A->addAttr(Attribute::NoCapture);
-        ++NumNoCapture;
-        Changed = true;
-      }
-      continue;
-    }
-
-    bool SCCCaptured = false;
-    for (auto I = ArgumentSCC.begin(), E = ArgumentSCC.end();
-         I != E && !SCCCaptured; ++I) {
-      ArgumentGraphNode *Node = *I;
-      if (Node->Uses.empty()) {
-        if (!Node->Definition->hasNoCaptureAttr())
-          SCCCaptured = true;
-      }
-    }
-    if (SCCCaptured)
-      continue;
-
-    SmallPtrSet<Argument *, 8> ArgumentSCCNodes;
-    // Fill ArgumentSCCNodes with the elements of the ArgumentSCC.  Used for
-    // quickly looking up whether a given Argument is in this ArgumentSCC.
-    for (ArgumentGraphNode *I : ArgumentSCC) {
-      ArgumentSCCNodes.insert(I->Definition);
-    }
-
-    for (auto I = ArgumentSCC.begin(), E = ArgumentSCC.end();
-         I != E && !SCCCaptured; ++I) {
-      ArgumentGraphNode *N = *I;
-      for (ArgumentGraphNode *Use : N->Uses) {
-        Argument *A = Use->Definition;
-        if (A->hasNoCaptureAttr() || ArgumentSCCNodes.count(A))
-          continue;
-        SCCCaptured = true;
-        break;
-      }
-    }
-    if (SCCCaptured)
-      continue;
-
-    for (unsigned i = 0, e = ArgumentSCC.size(); i != e; ++i) {
-      Argument *A = ArgumentSCC[i]->Definition;
-      A->addAttr(Attribute::NoCapture);
-      ++NumNoCapture;
-      Changed = true;
-    }
-
-    // We also want to compute readonly/readnone. With a small number of false
-    // negatives, we can assume that any pointer which is captured isn't going
-    // to be provably readonly or readnone, since by definition we can't
-    // analyze all uses of a captured pointer.
-    //
-    // The false negatives happen when the pointer is captured by a function
-    // that promises readonly/readnone behaviour on the pointer, then the
-    // pointer's lifetime ends before anything that writes to arbitrary memory.
-    // Also, a readonly/readnone pointer may be returned, but returning a
-    // pointer is capturing it.
-
-    Attribute::AttrKind ReadAttr = Attribute::ReadNone;
-    for (unsigned i = 0, e = ArgumentSCC.size(); i != e; ++i) {
-      Argument *A = ArgumentSCC[i]->Definition;
-      Attribute::AttrKind K = determinePointerReadAttrs(A, ArgumentSCCNodes);
-      if (K == Attribute::ReadNone)
-        continue;
-      if (K == Attribute::ReadOnly) {
-        ReadAttr = Attribute::ReadOnly;
-        continue;
-      }
-      ReadAttr = K;
-      break;
-    }
-
-    if (ReadAttr != Attribute::None) {
-      for (unsigned i = 0, e = ArgumentSCC.size(); i != e; ++i) {
-        Argument *A = ArgumentSCC[i]->Definition;
-        // Clear out existing readonly/readnone attributes
-        A->removeAttr(Attribute::ReadOnly);
-        A->removeAttr(Attribute::ReadNone);
-        A->addAttr(ReadAttr);
-        ReadAttr == Attribute::ReadOnly ? ++NumReadOnlyArg : ++NumReadNoneArg;
-        Changed = true;
-      }
-    }
   }
 
   return Changed;
 }
 
 /// Tests whether a function is "malloc-like".
 ///
 /// A function is "malloc-like" if it returns either null or a pointer that

llvm/test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll

 ; RUN: opt < %s -tbaa -basicaa -attributor -functionattrs -S | FileCheck %s
 
 ; FunctionAttrs should make use of TBAA.
 
 ; Add the readnone attribute, since the only access is a store which TBAA
 ; says is to constant memory.
 ;
 ; It's unusual to see a store to constant memory, but it isn't necessarily
 ; invalid, as it's possible that this only happens after optimization on a
 ; code path which isn't ever executed.
 
-; CHECK: define void @test0_yes(i32* nocapture %p) #0 {
+; CHECK: define void @test0_yes(i32* nocapture readnone %p) #0 {
 define void @test0_yes(i32* %p) nounwind {
   store i32 0, i32* %p, !tbaa !1
   ret void
 }
 
-; CHECK: define void @test0_no(i32* nocapture %p) #1 {
+; CHECK: define void @test0_no(i32* nocapture writeonly %p) #1 {
 define void @test0_no(i32* %p) nounwind {
   store i32 0, i32* %p, !tbaa !2
   ret void
 }
 
 ; Add the readonly attribute, since there's just a call to a function which 
 ; TBAA says doesn't modify any memory.
 
-; CHECK: define void @test1_yes(i32* nocapture %p) #2 {
+; CHECK: define void @test1_yes(i32* nocapture readonly %p) #2 {
 define void @test1_yes(i32* %p) nounwind {
   call void @callee(i32* %p), !tbaa !1
   ret void
 }
 
 ; CHECK: define void @test1_no(i32* %p) #3 {
 define void @test1_no(i32* %p) nounwind {
   call void @callee(i32* %p), !tbaa !2
   ret void
 }
 
 ; Add the readonly attribute, as above, but this time BasicAA will say
 ; that the function accesses memory through its arguments, which TBAA
 ; still says that the function doesn't write to memory.
 ;
 ; This is unusual, since the function is memcpy, but as above, this
 ; isn't necessarily invalid.
 
-; CHECK: define void @test2_yes(i8* nocapture %p, i8* nocapture %q, i64 %n) #0 {
+; CHECK: define void @test2_yes(i8* nocapture readnone %p, i8* nocapture readnone %q, i64 %n) #0 {
 define void @test2_yes(i8* %p, i8* %q, i64 %n) nounwind {
   call void @llvm.memcpy.p0i8.p0i8.i64(i8* %p, i8* %q, i64 %n, i1 false), !tbaa !1
   ret void
 }
 
-; CHECK: define void @test2_no(i8* nocapture %p, i8* nocapture readonly %q, i64 %n) #4 {
+; CHECK: define void @test2_no(i8* nocapture writeonly %p, i8* nocapture readonly %q, i64 %n) #4 {
 define void @test2_no(i8* %p, i8* %q, i64 %n) nounwind {
   call void @llvm.memcpy.p0i8.p0i8.i64(i8* %p, i8* %q, i64 %n, i1 false), !tbaa !2
   ret void
 }
 
 ; Similar to the others, va_arg only accesses memory through its operand.
 
-; CHECK: define i32 @test3_yes(i8* nocapture %p) #0 {
+; CHECK: define i32 @test3_yes(i8* nocapture readnone %p) #0 {
 define i32 @test3_yes(i8* %p) nounwind {
   %t = va_arg i8* %p, i32, !tbaa !1
   ret i32 %t
 }
 
 ; CHECK: define i32 @test3_no(i8* nocapture %p) #4 {
 define i32 @test3_no(i8* %p) nounwind {
   %t = va_arg i8* %p, i32, !tbaa !2

llvm/test/Feature/OperandBundles/function-attrs.ll

-; RUN: opt -S -functionattrs < %s | FileCheck %s
+; RUN: opt -S -functionattrs -attributor < %s | FileCheck %s
 
 declare void @f_readonly() readonly
 declare void @f_readnone() readnone
 
 define void @test_0(i32* %x) {
 ; FunctionAttrs must not infer readonly / readnone for %x
 
 ; CHECK-LABEL: define void @test_0(i32* %x) {

llvm/test/Other/cgscc-devirt-iteration.ll

 ; The CGSCC pass manager includes an SCC iteration utility that tracks indirect
 ; calls that are turned into direct calls (devirtualization) and re-visits the
 ; SCC to expose those calls to the SCC-based IPO passes. We trigger
 ; devirtualization here with GVN which forwards a store through a load and to
 ; an indirect call.
 ;
-; RUN: opt -aa-pipeline=basic-aa -passes='cgscc(function-attrs,function(gvn,instcombine))' -S < %s | FileCheck %s --check-prefix=CHECK --check-prefix=BEFORE
-; RUN: opt -aa-pipeline=basic-aa -passes='cgscc(devirt<1>(function-attrs,function(gvn,instcombine)))' -S < %s | FileCheck %s --check-prefix=CHECK --check-prefix=AFTER --check-prefix=AFTER1
-; RUN: opt -aa-pipeline=basic-aa -passes='cgscc(devirt<2>(function-attrs,function(gvn,instcombine)))' -S < %s | FileCheck %s --check-prefix=CHECK --check-prefix=AFTER --check-prefix=AFTER2
+; RUN: opt -aa-pipeline=basic-aa -passes='cgscc(attributor,function-attrs,function(gvn,instcombine))' -S < %s | FileCheck %s --check-prefix=CHECK --check-prefix=BEFORE
+; RUN: opt -aa-pipeline=basic-aa -passes='cgscc(devirt<1>(attributor,function-attrs,function(gvn,instcombine)))' -S < %s | FileCheck %s --check-prefix=CHECK --check-prefix=AFTER --check-prefix=AFTER1
+; RUN: opt -aa-pipeline=basic-aa -passes='cgscc(devirt<2>(attributor,function-attrs,function(gvn,instcombine)))' -S < %s | FileCheck %s --check-prefix=CHECK --check-prefix=AFTER --check-prefix=AFTER2
 ;
 ; We also verify that the real O2 pipeline catches these cases.
 ; RUN: opt -aa-pipeline=basic-aa -passes='default<O2>' -S < %s | FileCheck %s --check-prefix=CHECK --check-prefix=AFTER --check-prefix=AFTER2
 
 declare void @readnone() readnone
 ; CHECK: Function Attrs: readnone
 ; CHECK-NEXT: declare void @readnone()
 

 entry:
   %f1ptr = alloca void (void ()**)*
   store void (void ()**)* @readnone_with_arg, void (void ()**)** %f1ptr
   %f1 = load void (void ()**)*, void (void ()**)** %f1ptr
   ; This indirect call is the first to be resolved, allowing us to deduce
   ; readonly but not (yet) readnone.
   call void %f1(void ()** %ignore)
 ; CHECK: call void @readnone_with_arg(void ()** %ignore)
+  %cmp = icmp ne void ()** %ignore, null
+  br i1 %cmp, label %scc, label %end
 
-  ; Bogus call to test2_b to make this a cycle.
+scc:
+  ; Bogus call to test2_b to make this a cycle. Conditionally executed to
+  ; avoid no-return deduction potential consequences.
     call void @test2_b()
+    unreachable
 
+end:
   ret void
 }
 
 define void @test2_b() {
 ; BEFORE-NOT: Function Attrs
 ; AFTER1: Function Attrs: readonly
 ; AFTER2: Function Attrs: readnone
 ; CHECK-LABEL: define void @test2_b()

llvm/test/Transforms/FunctionAttrs/arg_returned.ll

return:                                           ; preds = %cond.end, %if.then3, %if.then
   %retval.0 = phi i32 [ %call1, %if.then ], [ %call11, %if.then3 ], [ %cond, %cond.end ]
   ret i32 %retval.0
 }
 
 
 ; TEST 2
 ;
 ; BOTH: define dso_local double* @ptr_sink_r0(double* readnone returned "no-capture-maybe-returned" %r) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
-; BOTH: define dso_local double* @ptr_scc_r1(double* %a, double* readnone returned %r, double* nocapture readnone %b) [[NoInlineNoUnwindReadnoneUwtable]]
-; BOTH: define dso_local double* @ptr_scc_r2(double* readnone %a, double* readnone %b, double* readnone returned %r) [[NoInlineNoUnwindReadnoneUwtable]]
+; BOTH: define dso_local double* @ptr_scc_r1(double* %a, double* returned %r, double* nocapture readnone %b) [[NoInlineNoUnwindReadnoneUwtable]]
+; BOTH: define dso_local double* @ptr_scc_r2(double* %a, double* %b, double* returned %r) [[NoInlineNoUnwindReadnoneUwtable]]
 ;
-; FNATTR: define dso_local double* @ptr_sink_r0(double* readnone %r) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
-; FNATTR: define dso_local double* @ptr_scc_r1(double* %a, double* readnone %r, double* nocapture readnone %b) [[NoInlineNoUnwindReadnoneUwtable]]
-; FNATTR: define dso_local double* @ptr_scc_r2(double* readnone %a, double* readnone %b, double* readnone %r) [[NoInlineNoUnwindReadnoneUwtable]]
+; FNATTR: define dso_local double* @ptr_sink_r0(double* %r) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
+; FNATTR: define dso_local double* @ptr_scc_r1(double* %a, double* %r, double* %b) [[NoInlineNoUnwindReadnoneUwtable]]
+; FNATTR: define dso_local double* @ptr_scc_r2(double* %a, double* %b, double* %r) [[NoInlineNoUnwindReadnoneUwtable]]
 ;
+; It would be acceptable for this example to mark all pointers as readnone.
 ; ATTRIBUTOR: define dso_local double* @ptr_sink_r0(double* readnone returned "no-capture-maybe-returned" %r) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
 ; ATTRIBUTOR: define dso_local double* @ptr_scc_r1(double* %a, double* returned %r, double* nocapture readnone %b) [[NoInlineNoUnwindReadnoneUwtable]]
 ; ATTRIBUTOR: define dso_local double* @ptr_scc_r2(double* %a, double* %b, double* returned %r) [[NoInlineNoUnwindReadnoneUwtable]]
 ;
 ; double* ptr_scc_r1(double* a, double* b, double* r);
 ; double* ptr_scc_r2(double* a, double* b, double* r);
 ;
 ; __attribute__((noinline)) double* ptr_sink_r0(double* r) {

 
 ; TEST 3
 ;
 ; int* ret0(int *a) {
 ;   return *a ? a : ret0(ret0(ret0(...ret0(a)...)));
 ; }
 ;
 ; FEW_IT:  define dso_local i32* @ret0(i32* %a)
-; FNATTR:  define dso_local i32* @ret0(i32* readonly %a) [[NoInlineNoUnwindUwtable:#[0-9]*]]
+; FNATTR:  define dso_local i32* @ret0(i32* %a) [[NoInlineNoUnwindUwtable:#[0-9]*]]
 ; BOTH:    define dso_local i32* @ret0(i32* readonly returned "no-capture-maybe-returned" %a) [[NoInlineNoReturnNoUnwindReadonlyUwtable:#[0-9]*]]
 define dso_local i32* @ret0(i32* %a) #0 {
 entry:
   %v = load i32, i32* %a, align 4
   %tobool = icmp ne i32 %v, 0
   %call = call i32* @ret0(i32* %a)
   %call1 = call i32* @ret0(i32* %call)
   %call2 = call i32* @ret0(i32* %call1)

 ;  int* calls_unknown_fn(int *r) {
 ;    unknown_fn(&calls_unknown_fn);
 ;    return r;
 ;  }
 ;
 ; BOTH: declare void @unknown_fn(i32* (i32*)*) [[NoInlineNoUnwindUwtable:#[0-9]*]]
 ;
 ; BOTH:       define dso_local i32* @calls_unknown_fn(i32* readnone returned "no-capture-maybe-returned" %r) [[NoInlineNoUnwindUwtable]]
-; FNATTR:     define dso_local i32* @calls_unknown_fn(i32* readnone %r) [[NoInlineNoUnwindUwtable:#[0-9]*]]
+; FNATTR:     define dso_local i32* @calls_unknown_fn(i32* %r) [[NoInlineNoUnwindUwtable:#[0-9]*]]
 ; ATTRIBUTOR: define dso_local i32* @calls_unknown_fn(i32* readnone returned "no-capture-maybe-returned" %r) [[NoInlineNoUnwindUwtable:#[0-9]*]]
 ;
 declare void @unknown_fn(i32* (i32*)*) #0
 
 define dso_local i32* @calls_unknown_fn(i32* %r) #0 {
   tail call void @unknown_fn(i32* (i32*)* nonnull @calls_unknown_fn)
   ret i32* %r
 }

 
 
 ; TEST 8
 ;
 ; double* bitcast(int* b) {
 ;   return (double*)b;
 ; }
 ;
-; FNATTR:     define dso_local double* @bitcast(i32* readnone %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
+; FNATTR:     define dso_local double* @bitcast(i32* %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
 ; ATTRIBUTOR: define dso_local double* @bitcast(i32* readnone returned "no-capture-maybe-returned" %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
 ; BOTH:       define dso_local double* @bitcast(i32* readnone returned "no-capture-maybe-returned" %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
 ;
 define dso_local double* @bitcast(i32* %b) #0 {
 entry:
   %bc0 = bitcast i32* %b to double*
   ret double* %bc0
 }
 
 
 ; TEST 9
 ;
 ; double* bitcasts_select_and_phi(int* b) {
 ;   double* x = b;
 ;   if (b == 0)
 ;     x = b;
 ;   return b != 0 ? b : x;
 ; }
 ;
-; FNATTR:     define dso_local double* @bitcasts_select_and_phi(i32* readnone %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
+; FNATTR:     define dso_local double* @bitcasts_select_and_phi(i32* %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
 ; ATTRIBUTOR: define dso_local double* @bitcasts_select_and_phi(i32* readnone returned "no-capture-maybe-returned" %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
 ; BOTH:       define dso_local double* @bitcasts_select_and_phi(i32* readnone returned "no-capture-maybe-returned" %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
 ;
 define dso_local double* @bitcasts_select_and_phi(i32* %b) #0 {
 entry:
   %bc0 = bitcast i32* %b to double*
   %cmp = icmp eq double* %bc0, null
   br i1 %cmp, label %if.then, label %if.end

 ; double* ret_arg_arg_undef(int* b) {
 ;   if (b == 0)
 ;     return (double*)b;
 ;   if (b == 0)
 ;     return (double*)b;
 ;   /* return undef */
 ; }
 ;
-; FNATTR:     define dso_local double* @ret_arg_arg_undef(i32* readnone %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
+; FNATTR:     define dso_local double* @ret_arg_arg_undef(i32* %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
 ; ATTRIBUTOR: define dso_local double* @ret_arg_arg_undef(i32* readnone returned "no-capture-maybe-returned" %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
 ; BOTH:       define dso_local double* @ret_arg_arg_undef(i32* readnone returned "no-capture-maybe-returned" %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
 ;
 define dso_local double* @ret_arg_arg_undef(i32* %b) #0 {
 entry:
   %bc0 = bitcast i32* %b to double*
   %cmp = icmp eq double* %bc0, null
   br i1 %cmp, label %ret_arg0, label %if.end

 ; double* ret_undef_arg_arg(int* b) {
 ;   if (b == 0)
 ;     return (double*)b;
 ;   if (b == 0)
 ;     return (double*)b;
 ;   /* return undef */
 ; }
 ;
-; FNATTR:     define dso_local double* @ret_undef_arg_arg(i32* readnone %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
+; FNATTR:     define dso_local double* @ret_undef_arg_arg(i32* %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
 ; ATTRIBUTOR: define dso_local double* @ret_undef_arg_arg(i32* readnone returned "no-capture-maybe-returned" %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
 ; BOTH:       define dso_local double* @ret_undef_arg_arg(i32* readnone returned "no-capture-maybe-returned" %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
 ;
 define dso_local double* @ret_undef_arg_arg(i32* %b) #0 {
 entry:
   %bc0 = bitcast i32* %b to double*
   %cmp = icmp eq double* %bc0, null
   br i1 %cmp, label %ret_undef, label %if.end

 ; double* ret_undef_arg_undef(int* b) {
 ;   if (b == 0)
 ;     /* return undef */
 ;   if (b == 0)
 ;     return (double*)b;
 ;   /* return undef */
 ; }
 ;
-; FNATTR:     define dso_local double* @ret_undef_arg_undef(i32* readnone %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
+; FNATTR:     define dso_local double* @ret_undef_arg_undef(i32* %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
 ; ATTRIBUTOR: define dso_local double* @ret_undef_arg_undef(i32* readnone returned "no-capture-maybe-returned" %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
 ; BOTH:       define dso_local double* @ret_undef_arg_undef(i32* readnone returned "no-capture-maybe-returned" %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
 ;
 define dso_local double* @ret_undef_arg_undef(i32* %b) #0 {
 entry:
   %bc0 = bitcast i32* %b to double*
   %cmp = icmp eq double* %bc0, null
   br i1 %cmp, label %ret_undef0, label %if.end

llvm/test/Transforms/FunctionAttrs/nocapture.ll

%bit = and i32 %tmp2, 1
         ; subtle escape mechanism follows
   %lookup = getelementptr [2 x i1], [2 x i1]* @lookup_table, i32 0, i32 %bit
   %val = load i1, i1* %lookup
   ret i1 %val
 }
 
 declare void @throw_if_bit_set(i8*, i8) readonly
 
-; CHECK: define i1 @c6(i8* readonly %q, i8 %bit)
+; FIXME: missing readonly for %q
+; CHECK: define i1 @c6(i8* %q, i8 %bit)
 define i1 @c6(i8* %q, i8 %bit) personality i32 (...)* @__gxx_personality_v0 {
   invoke void @throw_if_bit_set(i8* %q, i8 %bit)
     to label %ret0 unwind label %ret1
 ret0:
   ret i1 0
 ret1:
         %exn = landingpad {i8*, i32}
                  cleanup
   ret i1 1
 }
 
 declare i32 @__gxx_personality_v0(...)
 
 define i1* @lookup_bit(i32* %q, i32 %bitno) readnone nounwind {
   %tmp = ptrtoint i32* %q to i32
   %tmp2 = lshr i32 %tmp, %bitno
   %bit = and i32 %tmp2, 1
   %lookup = getelementptr [2 x i1], [2 x i1]* @lookup_table, i32 0, i32 %bit
   ret i1* %lookup
 }
 
-; CHECK: define i1 @c7(i32* readonly %q, i32 %bitno)
+; FIXME: missing readonly for %q
+; CHECK: define i1 @c7(i32* %q, i32 %bitno)
 define i1 @c7(i32* %q, i32 %bitno) {
   %ptr = call i1* @lookup_bit(i32* %q, i32 %bitno)
   %val = load i1, i1* %ptr
   ret i1 %val
 }
 
 
 ; CHECK: define i32 @nc1(i32* %q, i32* nocapture %p, i1 %b)

llvm/test/Transforms/FunctionAttrs/nonnull.ll

-; RUN: opt -S -functionattrs -enable-nonnull-arg-prop %s | FileCheck %s
-; RUN: opt -S -passes=function-attrs -enable-nonnull-arg-prop %s | FileCheck %s
+; RUN: opt -S -attributor -functionattrs -enable-nonnull-arg-prop %s | FileCheck %s
+; RUN: opt -S -passes='attributor,function-attrs' -enable-nonnull-arg-prop %s | FileCheck %s
 
 declare nonnull i8* @ret_nonnull()
 
 ; Return a pointer trivially nonnull (call return attribute)
 define i8* @test1() {
 ; CHECK: define nonnull i8* @test1
   %ret = call i8* @ret_nonnull()
   ret i8* %ret

llvm/test/Transforms/FunctionAttrs/norecurse.ll

 
 ; CHECK: Function Attrs
 ; CHECK-NEXT: declare i32 @k()
 declare i32 @k() readnone
 
 ; CHECK: Function Attrs
 ; CHECK-SAME: nounwind
 ; CHECK-NOT: norecurse
-; CHECK-NEXT: define void @intrinsic(i8* nocapture %dest, i8* nocapture readonly %src, i32 %len)
+; CHECK-NEXT: define void @intrinsic(i8* nocapture writeonly %dest, i8* nocapture readonly %src, i32 %len)
 define void @intrinsic(i8* %dest, i8* %src, i32 %len) {
   call void @llvm.memcpy.p0i8.p0i8.i32(i8* %dest, i8* %src, i32 %len, i1 false)
   ret void
 }
 
 ; CHECK: Function Attrs
 ; CHECK-NEXT: declare void @llvm.memcpy.p0i8.p0i8.i32
 declare void @llvm.memcpy.p0i8.p0i8.i32(i8*, i8*, i32, i1)

llvm/test/Transforms/FunctionAttrs/out-of-bounds-iterator-bug.ll

 ; RUN: opt -functionattrs -S < %s | FileCheck %s
 ; RUN: opt -passes=function-attrs -S < %s | FileCheck %s
 
 ; This checks for an iterator wraparound bug in FunctionAttrs.  The previous
 ; "incorrect" behavior was inferring readonly for the %x argument in @caller.
 ; Inferring readonly for %x *is* actually correct, since @va_func is marked
 ; readonly, but FunctionAttrs was inferring readonly for the wrong reasons (and
 ; we _need_ the readonly on @va_func to trigger the problematic code path).  It
 ; is possible that in the future FunctionAttrs becomes smart enough to infer
 ; readonly for %x for the right reasons, and at that point this test will have
 ; to be marked invalid.
 
 declare void @llvm.va_start(i8*)
 declare void @llvm.va_end(i8*)
 
+; FIXME: missing nocapture on %b
 define void @va_func(i32* readonly %b, ...) readonly nounwind {
-; CHECK-LABEL: define void @va_func(i32* nocapture readonly %b, ...)
+; CHECK-LABEL: define void @va_func(i32* readonly %b, ...)
  entry:
   %valist = alloca i8
   call void @llvm.va_start(i8* %valist)
   call void @llvm.va_end(i8* %valist)
   %x = call i32 @caller(i32* %b)
   ret void
 }
 
 define i32 @caller(i32* %x) {
 ; CHECK-LABEL: define i32 @caller(i32* nocapture %x)
  entry:
   call void(i32*,...) @va_func(i32* null, i32 0, i32 0, i32 0, i32* %x)
   ret i32 42
 }

llvm/test/Transforms/FunctionAttrs/readattrs.ll

 ; RUN: opt < %s -attributor -functionattrs -S | FileCheck %s
 ; RUN: opt < %s -aa-pipeline=basic-aa -passes='attributor,cgscc(function-attrs)' -S | FileCheck %s
 @x = global i32 0
 
 declare void @test1_1(i8* %x1_1, i8* readonly %y1_1, ...)
 
 ; NOTE: readonly for %y1_2 would be OK here but not for the similar situation in test13.
 ;
-; CHECK: define void @test1_2(i8* %x1_2, i8* readonly %y1_2, i8* %z1_2)
+; CHECK: define void @test1_2(i8* %x1_2, i8* %y1_2, i8* %z1_2)
 define void @test1_2(i8* %x1_2, i8* %y1_2, i8* %z1_2) {
   call void (i8*, i8*, ...) @test1_1(i8* %x1_2, i8* %y1_2, i8* %z1_2)
   store i32 0, i32* @x
   ret void
 }
 
 ; CHECK: define i8* @test2(i8* readnone returned "no-capture-maybe-returned" %p)
 define i8* @test2(i8* %p) {
   store i32 0, i32* @x
   ret i8* %p
 }
 
-; CHECK: define i1 @test3(i8* readnone %p, i8* readnone %q)
+; We should derive readnone from the function behavior once available in the attributor
+;   i1 @test3(i8* readnone %p, i8* readnone %q)
+; CHECK: define i1 @test3(i8* %p, i8* %q)
 define i1 @test3(i8* %p, i8* %q) {
   %A = icmp ult i8* %p, %q
   ret i1 %A
 }
 
 declare void @test4_1(i8* nocapture) readonly
 
 ; CHECK: define void @test4_2(i8* nocapture readonly %p)

 declare void @escape_readonly_ptr(i8** %addr, i8* readnone %ptr)
 declare void @escape_readnone_ptr(i8** %addr, i8* readonly %ptr)
 
 ; The argument pointer %escaped_then_written cannot be marked readnone/only even
 ; though the only direct use, in @escape_readnone_ptr/@escape_readonly_ptr,
 ; is marked as readnone/only. However, the functions can write the pointer into
 ; %addr, causing the store to write to %escaped_then_written.
 ;
-; FIXME: This test currently exposes a bug!
-;
-; BUG: define void @unsound_readnone(i8* %ignored, i8* readnone %escaped_then_written)
-; BUG: define void @unsound_readonly(i8* %ignored, i8* readonly %escaped_then_written)
+; CHECK: define void @unsound_readnone(i8* %ignored, i8* %escaped_then_written)
+; CHECK: define void @unsound_readonly(i8* %ignored, i8* %escaped_then_written)
 define void @unsound_readnone(i8* %ignored, i8* %escaped_then_written) {
   %addr = alloca i8*
   call void @escape_readnone_ptr(i8** %addr, i8* %escaped_then_written)
   %addr.ld = load i8*, i8** %addr
   store i8 0, i8* %addr.ld
   ret void
 }
 
 define void @unsound_readonly(i8* %ignored, i8* %escaped_then_written) {
   %addr = alloca i8*
   call void @escape_readonly_ptr(i8** %addr, i8* %escaped_then_written)
   %addr.ld = load i8*, i8** %addr
   store i8 0, i8* %addr.ld
   ret void
 }

llvm/test/Transforms/FunctionAttrs/readnone.ll

-; RUN: opt < %s -functionattrs -S | FileCheck %s
-; RUN: opt < %s -passes=function-attrs -S | FileCheck %s
+; RUN: opt < %s -attributor -S | FileCheck %s
+; RUN: opt < %s -passes=attributor -S | FileCheck %s
 
 ; CHECK: define void @bar(i8* nocapture readnone)
 define void @bar(i8* readonly) {
   call void @foo(i8* %0)
     ret void
 }
 
 ; CHECK: define void @foo(i8* nocapture readnone)
 define void @foo(i8* readonly) {
   call void @bar(i8* %0)
   ret void
 }