[llvm] 43907d6 - Fix incorrect inference of writeonly despite reading operand bundle

[Philip Reames via llvm-commits]

Author: Philip Reames
Date: 2022-01-18T12:34:18-08:00
New Revision: 43907d608a22a61245813efeb978928fdfb90b6a

URL: https://github.com/llvm/llvm-project/commit/43907d608a22a61245813efeb978928fdfb90b6a
DIFF: https://github.com/llvm/llvm-project/commit/43907d608a22a61245813efeb978928fdfb90b6a.diff

LOG: Fix incorrect inference of writeonly despite reading operand bundle

If we have a writeonly function called from a callsite with a potentially reading operand bundle, we can not conclude the callsite is writeonly.

The changed test is the only one I've been able to demonstrate a current miscompile on, but an incorrect result here could show up in a bunch of subtle ways.  For instance, this issue caused several spurious test changes when combined with D117591.

Added: 
    

Modified: 
    llvm/include/llvm/IR/InstrTypes.h
    llvm/test/Feature/OperandBundles/function-attrs.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/IR/InstrTypes.h b/llvm/include/llvm/IR/InstrTypes.h
index 03839e00c7aab..677137271974f 100644
--- a/llvm/include/llvm/IR/InstrTypes.h
+++ b/llvm/include/llvm/IR/InstrTypes.h
@@ -2087,8 +2087,8 @@ class CallBase : public Instruction {
   /// Is the function attribute S disallowed by some operand bundle on
   /// this operand bundle user?
   bool isFnAttrDisallowedByOpBundle(StringRef S) const {
-    // Operand bundles only possibly disallow readnone, readonly and argmemonly
-    // attributes.  All String attributes are fine.
+    // Operand bundles only possibly disallow memory access attributes.  All
+    // String attributes are fine.
     return false;
   }
 
@@ -2113,6 +2113,9 @@ class CallBase : public Instruction {
 
     case Attribute::ReadOnly:
       return hasClobberingOperandBundles();
+
+    case Attribute::WriteOnly:
+      return hasReadingOperandBundles();
     }
 
     llvm_unreachable("switch has a default case!");

diff  --git a/llvm/test/Feature/OperandBundles/function-attrs.ll b/llvm/test/Feature/OperandBundles/function-attrs.ll
index a27daef5fca39..b0c60e1ab89b9 100644
--- a/llvm/test/Feature/OperandBundles/function-attrs.ll
+++ b/llvm/test/Feature/OperandBundles/function-attrs.ll
@@ -24,11 +24,10 @@ define void @test_1(i32* %x) {
   ret void
 }
 
-; FIXME: We are incorectly inferring writeonly on the function
 define void @test_2(i32* %x) {
 ; FunctionAttrs must not infer writeonly
 
-; CHECK-LABEL: define void @test_2(i32* %x) #2 {
+; CHECK-LABEL: define void @test_2(i32* %x) {
  entry:
  ; CHECK: call void @f_writeonly() [ "foo"(i32* %x) ]
   call void @f_writeonly() [ "foo"(i32* %x) ]