[funcattrs] Infer writeonly argument attribute

[reames]

Summary

This change extends the current logic for inferring readonly and readnone argument attributes to also infer writeonly.

This change is deliberately minimal; there's a couple of areas for follow up.

1. I left out all call handling and thus any benefit from the SCC walk. When examining the test changes, I realized the existing code is imprecise, and am going to fix that in it's own revision before adding in the writeonly handling. (Mostly because updating the tests is hard when I, the human, can't figure out whether the result is correct.)
2. I left out handling for storing a value (as opposed to storing to a pointer). This should benefit readonly/readnone as well, and applies to a bunch of other instructions. Seemed worth having as a separate review.

[aeubanks]

some clang tests are failing

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

 
 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(NumReturned, "Number of arguments marked returned");
 STATISTIC(NumReadNoneArg, "Number of arguments marked readnone");
 STATISTIC(NumReadOnlyArg, "Number of arguments marked readonly");
+STATISTIC(NumWriteOnlyArg, "Number of arguments marked writeonly");
 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");
 STATISTIC(NumNoFree, "Number of functions marked as nofree");
 STATISTIC(NumWillReturn, "Number of functions marked as willreturn");
 STATISTIC(NumNoSync, "Number of functions marked as nosync");
 
struct GraphTraits<ArgumentGraph *> : public GraphTraits<ArgumentGraphNode *> {
 
   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,
+determinePointerAccessAttrs(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() || A->hasPreallocatedAttr())
     return Attribute::None;
 
   bool IsRead = false;
-  // We don't need to track IsWritten. If A is written to, return immediately.
+  bool IsWrite = false;
 
   for (Use &U : A->uses()) {
     Visited.insert(&U);
     Worklist.push_back(&U);
   }
 
   while (!Worklist.empty()) {
+    if (IsWrite && IsRead)
+      // No point in searching further..
+      return Attribute::None;
+
     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::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::Store:
+      // A volatile store has side effects beyond what writeonly can be relied
+      // upon.
+      if (cast<StoreInst>(I)->isVolatile())
+        return Attribute::None;
+
+      IsWrite = true;
+      break;
+
     case Instruction::ICmp:
     case Instruction::Ret:
       break;
 
     default:
       return Attribute::None;
     }
   }
 
-  return IsRead ? Attribute::ReadOnly : Attribute::ReadNone;
+  if (IsWrite && IsRead)
+    return Attribute::None;
+  else if (IsRead)
+    return Attribute::ReadOnly;
+  else if (IsWrite)
+    return Attribute::WriteOnly;
+  else
+    return Attribute::ReadNone;
 }
 
 /// Deduce returned attributes for the SCC.
 static void addArgumentReturnedAttrs(const SCCNodeSet &SCCNodes,
                                      SmallSet<Function *, 8> &Changed) {
   // Check each function in turn, determining if an argument is always returned.
   for (Function *F : SCCNodes) {
     // We can infer and propagate function attributes only when we know that the
for (Instruction &I : Entry) {
     }
     if (!isGuaranteedToTransferExecutionToSuccessor(&I))
       break;
   }
 
   return Changed;
 }
 
-static bool addReadAttr(Argument *A, Attribute::AttrKind R) {
-  assert((R == Attribute::ReadOnly || R == Attribute::ReadNone)
-         && "Must be a Read attribute.");
+static bool addAccessAttr(Argument *A, Attribute::AttrKind R) {
+  assert((R == Attribute::ReadOnly || R == Attribute::ReadNone ||
+          R == Attribute::WriteOnly)
+         && "Must be an access attribute.");
   assert(A && "Argument must not be null.");
 
   // If the argument already has the attribute, nothing needs to be done.
   if (A->hasAttribute(R))
       return false;
 
   // Otherwise, remove potentially conflicting attribute, add the new one,
   // and update statistics.
   A->removeAttr(Attribute::WriteOnly);
   A->removeAttr(Attribute::ReadOnly);
   A->removeAttr(Attribute::ReadNone);
   A->addAttr(R);
-  R == Attribute::ReadOnly ? ++NumReadOnlyArg : ++NumReadNoneArg;
+  if (R == Attribute::ReadOnly)
+    ++NumReadOnlyArg;
+  else if (R == Attribute::WriteOnly)
+    ++NumWriteOnlyArg;
+  else
+    ++NumReadNoneArg;
   return true;
 }
 
 /// Deduce nocapture attributes for the SCC.
 static void addArgumentAttrs(const SCCNodeSet &SCCNodes,
                              SmallSet<Function *, 8> &Changed) {
   ArgumentGraph AG;
 
for (Function::arg_iterator A = F->arg_begin(), E = F->arg_end(); A != E;
               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.
+        // Can we determine that it's readonly/readnone/writeonly 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);
+        Attribute::AttrKind R = determinePointerAccessAttrs(&*A, Self);
         if (R != Attribute::None)
-          if (addReadAttr(A, R))
+          if (addAccessAttr(A, R))
             Changed.insert(F);
       }
     }
   }
 
   // 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
for (scc_iterator<ArgumentGraph *> I = scc_begin(&AG); !I.isAtEnd(); ++I) {
 
     for (unsigned i = 0, e = ArgumentSCC.size(); i != e; ++i) {
       Argument *A = ArgumentSCC[i]->Definition;
       A->addAttr(Attribute::NoCapture);
       ++NumNoCapture;
       Changed.insert(A->getParent());
     }
 
-    // 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.
+    // We also want to compute readonly/readnone/writeonly. 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) {
+    auto meetAccessAttr = [](Attribute::AttrKind A, Attribute::AttrKind B) {
+      if (A == B)
+        return A;
+      if (A == Attribute::ReadNone)
+        return B;
+      if (B == Attribute::ReadNone)
+        return A;
+      return Attribute::None;
+    };
+
+    Attribute::AttrKind AccessAttr = Attribute::ReadNone;
+    for (unsigned i = 0, e = ArgumentSCC.size();
+         i != e && AccessAttr != Attribute::None; ++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;
+      Attribute::AttrKind K = determinePointerAccessAttrs(A, ArgumentSCCNodes);
+      AccessAttr = meetAccessAttr(AccessAttr, K);
     }
 
-    if (ReadAttr != Attribute::None) {
+    if (AccessAttr != Attribute::None) {
       for (unsigned i = 0, e = ArgumentSCC.size(); i != e; ++i) {
         Argument *A = ArgumentSCC[i]->Definition;
-        if (addReadAttr(A, ReadAttr))
+        if (addAccessAttr(A, AccessAttr))
           Changed.insert(A->getParent());
       }
     }
   }
 }
 
 /// Tests whether a function is "malloc-like".
 ///

llvm/test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll

 ; code path which isn't ever executed.
 
 ; CHECK: define void @test0_yes(i32* nocapture %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.
 

llvm/test/Feature/OperandBundles/pr26510.ll

 ; RUN: opt -S -globals-aa -function-attrs < %s | FileCheck %s
 ; RUN: opt -S -O3 < %s | FileCheck %s
 
 ; Apart from checking for the direct cause of the bug, we also check
 ; if any problematic aliasing rules have accidentally snuck into -O3.
 ;
 ; Since the "abc" operand bundle is not a special operand bundle that
 ; LLVM knows about, all of the stores and loads in @test below have to
 ; stay.
 
 declare void @foo() readnone
 
-; CHECK-LABEL: define i8* @test(i8* %p)
+; CHECK-LABEL: define i8* @test(i8* writeonly %p)
 ; CHECK:   %a = alloca i8*, align 8
 ; CHECK:   store i8* %p, i8** %a, align 8
 ; CHECK:   call void @foo() [ "abc"(i8** %a) ]
 ; CHECK:   %reload = load i8*, i8** %a, align 8
 ; CHECK:   ret i8* %reload
 ; CHECK: }
 
 define i8* @test(i8* %p) {
   %a = alloca i8*, align 8
   store i8* %p, i8** %a, align 8
   call void @foo() ["abc" (i8** %a)]
   %reload = load i8*, i8** %a, align 8
   ret i8* %reload
 }

llvm/test/Transforms/Coroutines/coro-async.ll

entry:
   %continuation_actor_arg = extractvalue {i8*, i8*, i8*} %res.2, 1
   %actor.2 =  bitcast i8* %continuation_actor_arg to %async.actor*
 
   tail call swiftcc void @asyncReturn(i8* %async.ctxt, %async.task* %task.2, %async.actor* %actor.2)
   call i1 @llvm.coro.end(i8* %hdl, i1 0)
   unreachable
 }
 
-; CHECK-LABEL: define swiftcc void @my_async_function2(%async.task* %task, %async.actor* %actor, i8* %async.ctxt)
+; CHECK-LABEL: define swiftcc void @my_async_function2(%async.task* %task, %async.actor* %actor, i8* writeonly %async.ctxt)
 ; CHECK-SAME: #[[FRAMEPOINTER:[0-9]+]]
 ; CHECK-SAME: !dbg ![[SP3:[0-9]+]]
 ; CHECK: store i8* %async.ctxt,
 ; CHECK: store %async.actor* %actor,
 ; CHECK: store %async.task* %task,
 ; CHECK: [[CALLEE_CTXT:%.*]] =  tail call i8* @llvm.coro.async.context.alloc(
 ; CHECK: store i8* [[CALLEE_CTXT]],
 ; CHECK: store i8* bitcast (void (i8*, i8*, i8*)* @my_async_function2.resume.0 to i8*),
 ; CHECK: store i8* %async.ctxt,
 ; CHECK: tail call swiftcc void @asyncSuspend(i8* [[CALLEE_CTXT]], %async.task* %task, %async.actor* %actor)
 ; CHECK: ret void
 
-; CHECK-LABEL: define internal swiftcc void @my_async_function2.resume.0(i8* %0, i8* nocapture readnone %1, i8* nocapture readonly %2)
+; CHECK-LABEL: define internal swiftcc void @my_async_function2.resume.0(i8* writeonly %0, i8* nocapture readnone %1, i8* nocapture readonly %2)
 ; CHECK-SAME: #[[FRAMEPOINTER]]
 ; CHECK-SAME: !dbg ![[SP4:[0-9]+]]
 ; CHECK: [[CALLEE_CTXT_ADDR:%.*]] = bitcast i8* %2 to i8**
 ; CHECK: [[CALLEE_CTXT:%.*]] = load i8*, i8** [[CALLEE_CTXT_ADDR]]
 ; CHECK: [[CALLEE_CTXT_SPILL_ADDR:%.*]] = getelementptr inbounds i8, i8* [[CALLEE_CTXT]], i64 152
 ; CHECK: [[CALLEE_CTXT_SPILL_ADDR2:%.*]] = bitcast i8* [[CALLEE_CTXT_SPILL_ADDR]] to i8**
 ; CHECK: store i8* bitcast (void (i8*, i8*, i8*)* @my_async_function2.resume.1 to i8*),
 ; CHECK: [[CALLLE_CTXT_RELOAD:%.*]] = load i8*, i8** [[CALLEE_CTXT_SPILL_ADDR2]]

llvm/test/Transforms/FunctionAttrs/2009-01-02-LocalStores.ll

 ; RUN: opt < %s -function-attrs -S | FileCheck %s
 ; RUN: opt < %s -passes=function-attrs -S | FileCheck %s
 
 ; CHECK: define i32* @a(i32** nocapture readonly %p)
 define i32* @a(i32** %p) {
   %tmp = load i32*, i32** %p
   ret i32* %tmp
 }
 
-; CHECK: define i32* @b(i32* %q)
+; CHECK: define i32* @b(i32* writeonly %q)
 define i32* @b(i32 *%q) {
   %mem = alloca i32*
   store i32* %q, i32** %mem
   %tmp = call i32* @a(i32** %mem)
   ret i32* %tmp
 }
 
 ; CHECK: define i32* @c(i32* readnone returned %r)
 @g = global i32 0
 define i32* @c(i32 *%r) {
   %a = icmp eq i32* %r, null
   store i32 1, i32* @g
   ret i32* %r
 }

llvm/test/Transforms/FunctionAttrs/nocapture.ll

 ; RUN: opt -function-attrs -S < %s | FileCheck %s --check-prefixes=FNATTR
 ; RUN: opt -passes=function-attrs -S < %s | FileCheck %s --check-prefixes=FNATTR
 
 @g = global i32* null   ; <i32**> [#uses=1]
 
 ; FNATTR: define i32* @c1(i32* readnone returned %q)
 define i32* @c1(i32* %q) {
   ret i32* %q
 }
 
-; FNATTR: define void @c2(i32* %q)
+; FNATTR: define void @c2(i32* writeonly %q)
 ; It would also be acceptable to mark %q as readnone. Update @c3 too.
 define void @c2(i32* %q) {
   store i32* %q, i32** @g
   ret void
 }
 
 ; FNATTR: define void @c3(i32* %q)
 define void @c3(i32* %q) {

 @g2 = global i8* null
 ; FNATTR: define void @captureLaunder(i8* %p)
 define void @captureLaunder(i8* %p) {
   %b = call i8* @llvm.launder.invariant.group.p0i8(i8* %p)
   store i8* %b, i8** @g2
   ret void
 }
 
-; FNATTR: @nocaptureStrip(i8* nocapture %p)
+; FNATTR: @nocaptureStrip(i8* nocapture writeonly %p)
 define void @nocaptureStrip(i8* %p) {
 entry:
   %b = call i8* @llvm.strip.invariant.group.p0i8(i8* %p)
   store i8 42, i8* %b
   ret void
 }
 
 @g3 = global i8* null
-; FNATTR: define void @captureStrip(i8* %p)
+; FNATTR: define void @captureStrip(i8* writeonly %p)
 define void @captureStrip(i8* %p) {
   %b = call i8* @llvm.strip.invariant.group.p0i8(i8* %p)
   store i8* %b, i8** @g3
   ret void
 }
 
 ; FNATTR: define i1 @captureICmp(i32* readnone %x)
 define i1 @captureICmp(i32* %x) {

llvm/test/Transforms/FunctionAttrs/readattrs.ll

 declare void @test4_1(i8* nocapture) readonly
 
 ; CHECK: define void @test4_2(i8* nocapture readonly %p)
 define void @test4_2(i8* %p) {
   call void @test4_1(i8* %p)
   ret void
 }
 
-; CHECK: define void @test5(i8** nocapture %p, i8* %q)
+; CHECK: define void @test5(i8** nocapture writeonly %p, i8* writeonly %q)
 ; Missed optz'n: we could make %q readnone, but don't break test6!
 define void @test5(i8** %p, i8* %q) {
   store i8* %q, i8** %p
   ret void
 }
 
 declare void @test6_1()
-; CHECK: define void @test6_2(i8** nocapture %p, i8* %q)
+; CHECK: define void @test6_2(i8** nocapture writeonly %p, i8* writeonly %q)
 ; This is not a missed optz'n.
 define void @test6_2(i8** %p, i8* %q) {
   store i8* %q, i8** %p
   call void @test6_1()
   ret void
 }
 
 ; CHECK: define void @test7_1(i32* nocapture inalloca(i32) %a)

 }
 
 ; CHECK: define i32* @test8_1(i32* readnone returned %p)
 define i32* @test8_1(i32* %p) {
 entry:
   ret i32* %p
 }
 
-; CHECK: define void @test8_2(i32* %p)
+; CHECK: define void @test8_2(i32* writeonly %p)
 define void @test8_2(i32* %p) {
 entry:
   %call = call i32* @test8_1(i32* %p)
   store i32 10, i32* %call, align 4
   ret void
 }
 
 ; CHECK: declare void @llvm.masked.scatter

llvm/test/Transforms/FunctionAttrs/writeonly.ll

 
 ; CHECK: define void @nouses-argworn-funwo(i32* nocapture readnone %.aaa) #2 {
 define void @nouses-argworn-funwo(i32* writeonly %.aaa) {
 nouses-argworn-funwo_entry:
   store i8 0, i8* getelementptr inbounds (%_type_of_d-ccc, %_type_of_d-ccc* @d-ccc, i32 0, i32 3)
   ret void
 }
 
-; CHECK: define void @test_store(i8* nocapture %p)
+; CHECK: define void @test_store(i8* nocapture writeonly %p)
 define void @test_store(i8* %p) {
   store i8 0, i8* %p
   ret void
 }
 
-; CHECK: define void @test_addressing(i8* nocapture %p)
+; CHECK: define void @test_addressing(i8* nocapture writeonly %p)
 define void @test_addressing(i8* %p) {
   %gep = getelementptr i8, i8* %p, i64 8
   %bitcast = bitcast i8* %gep to i32*
   store i32 0, i32* %bitcast
   ret void
 }
 
 ; CHECK: define void @test_readwrite(i8* nocapture %p)