From 5005c73f9003e68db39216812d55ac775fe6b48b Mon Sep 17 00:00:00 2001
From: Whitney Tsang <whitney.uwaterloo@gmail.com>
Date: Wed, 11 Sep 2019 14:26:22 +0000
Subject: [PATCH] LLVM: Optimization Pass: Remove conflicting attribute, if
 any, before adding new read attribute to an argument Summary: Update
 optimization pass to prevent adding read-attribute to an argument without
 removing its conflicting attribute.

A read attribute, based on the result of the attribute deduction
process, might be added to an argument. The attribute might be in
conflict with other read/write attribute currently associated with the
argument. To ensure the compatibility of attributes, conflicting
attribute, if any, must be removed before a new one is added.

The following snippet shows the current behavior of the compiler, where
the compilation process is aborted due to incompatible attributes.

$ cat x.ll
; ModuleID = 'x.bc'

%_type_of_d-ccc = type <{ i8*, i8, i8, i8, i8 }>

@d-ccc = internal global %_type_of_d-ccc <{ i8* null, i8 1, i8 13, i8 0,
i8 -127 }>, align 8

define void @foo(i32* writeonly %.aaa) {
foo_entry:
  %_param_.aaa = alloca i32*, align 8
  store i32* %.aaa, i32** %_param_.aaa, align 8
  store i8 0, i8* getelementptr inbounds (%_type_of_d-ccc,
%_type_of_d-ccc* @d-ccc, i32 0, i32 3)
  ret void
}

$ opt -O3 x.ll
Attributes 'readnone and writeonly' are incompatible!
void (i32*)* @foo
in function foo
LLVM ERROR: Broken function found, compilation aborted!
The purpose of this changeset is to fix the above error. This fix is
based on a suggestion from Johannes @jdoerfert (many thanks!!!)
Authored By: anhtuyen
Reviewer: nicholas, rnk, chandlerc, jdoerfert
Reviewed By: rnk
Subscribers: hiraditya, jdoerfert, llvm-commits, anhtuyen, LLVM
Tag: LLVM
Differential Revision: https://reviews.llvm.org/D58694

llvm-svn: 371622
---
 lib/Transforms/IPO/FunctionAttrs.cpp       | 33 ++++++++++++++--------
 test/Transforms/FunctionAttrs/writeonly.ll | 30 ++++++++++++++++++++
 2 files changed, 52 insertions(+), 11 deletions(-)
 create mode 100644 test/Transforms/FunctionAttrs/writeonly.ll

diff --git a/lib/Transforms/IPO/FunctionAttrs.cpp b/lib/Transforms/IPO/FunctionAttrs.cpp
index 5ccd8bc4b0f..0e923a717e6 100644
--- a/lib/Transforms/IPO/FunctionAttrs.cpp
+++ b/lib/Transforms/IPO/FunctionAttrs.cpp
@@ -664,6 +664,25 @@ static bool addArgumentAttrsFromCallsites(Function &F) {
   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 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;
+  return true;
+}
+
 /// Deduce nocapture attributes for the SCC.
 static bool addArgumentAttrs(const SCCNodeSet &SCCNodes) {
   bool Changed = false;
@@ -732,11 +751,8 @@ static bool addArgumentAttrs(const SCCNodeSet &SCCNodes) {
         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 +849,7 @@ static bool addArgumentAttrs(const SCCNodeSet &SCCNodes) {
     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);
       }
     }
   }
diff --git a/test/Transforms/FunctionAttrs/writeonly.ll b/test/Transforms/FunctionAttrs/writeonly.ll
new file mode 100644
index 00000000000..6514cd9d106
--- /dev/null
+++ b/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 }>
+
+@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 }