[PATCH] D58694: LLVM: Optimization Pass: Remove conflicting attribute, if any, before adding new read attribute to an argument

[Anh Tuyen Tran via Phabricator via llvm-commits]

anhtuyen updated this revision to Diff 217692.
anhtuyen retitled this revision from "LLVM: Optimization Pass: Function Attribute: Fix error caused by adding incompactible attribute to WriteOnly attribute of pointer argument" to "LLVM: Optimization Pass: Remove conflicting attribute, if any, before adding new read attribute to an argument".
anhtuyen edited the summary of this revision.
anhtuyen added a comment.

My apologies about the long pause before providing this patch. I was on a long vacation, and was then driven into some other issues until now. This new patch will **remove conflicting attribute**, if any, currently associated with an argument before adding a **read attribute** to it. It also updates the statistics accordingly.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58694/new/

https://reviews.llvm.org/D58694

Files:
  llvm/lib/Transforms/IPO/FunctionAttrs.cpp
  llvm/test/Transforms/FunctionAttrs/writeonly.ll


Index: llvm/test/Transforms/FunctionAttrs/writeonly.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/FunctionAttrs/writeonly.ll
@@ -0,0 +1,30 @@
+; RUN: opt < %s -functionattrs         -S | FileCheck %s
+; RUN: opt < %s -passes=function-attrs -S | FileCheck %s
+
+; CHECK: define void @nouses-argworn-funrn(i32* nocapture readnone %.aaa) #0 {
+define void @nouses-argworn-funrn(i32* writeonly %.aaa) {
+nouses-argworn-funrn_entry:
+  ret void
+}
+
+; CHECK: define void @nouses-argworn-funro(i32* nocapture readnone %.aaa, i32* nocapture readonly %.bbb) #1 {
+define void @nouses-argworn-funro(i32* writeonly %.aaa, i32* %.bbb) {
+nouses-argworn-funro_entry:
+  %val = load i32 , i32* %.bbb
+  ret void
+}
+
+%_type_of_d-ccc = type <{ i8*, i8, i8, i8, i8 }>
+
+ at d-ccc = internal global %_type_of_d-ccc <{ i8* null, i8 1, i8 13, i8 0, i8 -127 }>, align 8
+
+; 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: attributes #0 = { {{.*}} readnone }
+; CHECK: attributes #1 = { {{.*}} readonly }
+; CHECK: attributes #2 = { {{.*}} writeonly }
Index: llvm/lib/Transforms/IPO/FunctionAttrs.cpp
===================================================================
--- llvm/lib/Transforms/IPO/FunctionAttrs.cpp
+++ llvm/lib/Transforms/IPO/FunctionAttrs.cpp
@@ -664,6 +664,30 @@
   return Changed;
 }
 
+static bool addReadAttr(Argument *A, Attribute::AttrKind R) {
+  assert((R == Attribute::ReadOnly || R == Attribute::ReadNone)
+         && "Must be a Read 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 the conflicting attribute, update the statistics.
+  if (A->hasAttribute(Attribute::WriteOnly))
+    A->removeAttr(Attribute::WriteOnly);
+  else if (A->hasAttribute(Attribute::ReadOnly)) {
+    A->removeAttr(Attribute::ReadOnly);
+    --NumReadOnlyArg;
+  } else if (A->hasAttribute(Attribute::ReadNone)) {
+    A->removeAttr(Attribute::ReadNone);
+    --NumReadNoneArg;
+  }
+  A->addAttr(R);
+  R == Attribute::ReadOnly ? ++NumReadOnlyArg : ++NumReadNoneArg;
+  return true;
+}
+
 /// Deduce nocapture attributes for the SCC.
 static bool addArgumentAttrs(const SCCNodeSet &SCCNodes) {
   bool Changed = false;
@@ -732,11 +756,8 @@
         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;
-        }
+        if (R != Attribute::None)
+          Changed = addReadAttr(A, R);
       }
     }
   }
@@ -833,12 +854,7 @@
     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;
+        Changed = addReadAttr(A, ReadAttr);
       }
     }
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D58694.217692.patch
Type: text/x-patch
Size: 3540 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190828/4233ae0f/attachment.bin>