[llvm] 31c0e52 - A readonly operand bundle should not prevent inference of readonly from a readnone callee

[Philip Reames via llvm-commits]

Author: Philip Reames
Date: 2022-01-18T12:55:13-08:00
New Revision: 31c0e52420bb089291758a11e65e1672244c2e9f

URL: https://github.com/llvm/llvm-project/commit/31c0e52420bb089291758a11e65e1672244c2e9f
DIFF: https://github.com/llvm/llvm-project/commit/31c0e52420bb089291758a11e65e1672244c2e9f.diff

LOG: A readonly operand bundle should not prevent inference of readonly from a readnone callee

A readonly operand bundle disallows inference of readnone from the callee, but it should not prevent us from using the readnone fact on the callee to infer readonly for the callsite.

Fixes pr53270.

Differential Revision: https://reviews.llvm.org/D117591

Added: 
    

Modified: 
    llvm/include/llvm/IR/InstrTypes.h
    llvm/test/Transforms/InstCombine/trivial-dse-calls.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/IR/InstrTypes.h b/llvm/include/llvm/IR/InstrTypes.h
index 677137271974f..179aa579fa96b 100644
--- a/llvm/include/llvm/IR/InstrTypes.h
+++ b/llvm/include/llvm/IR/InstrTypes.h
@@ -1818,14 +1818,14 @@ class CallBase : public Instruction {
 
   /// Determine if the call does not access or only reads memory.
   bool onlyReadsMemory() const {
-    return doesNotAccessMemory() || hasFnAttr(Attribute::ReadOnly);
+    return hasImpliedFnAttr(Attribute::ReadOnly);
   }
 
   void setOnlyReadsMemory() { addFnAttr(Attribute::ReadOnly); }
 
   /// Determine if the call does not access or only writes memory.
   bool onlyWritesMemory() const {
-    return doesNotAccessMemory() || hasFnAttr(Attribute::WriteOnly);
+    return hasImpliedFnAttr(Attribute::WriteOnly);
   }
   void setOnlyWritesMemory() { addFnAttr(Attribute::WriteOnly); }
 
@@ -2288,6 +2288,26 @@ class CallBase : public Instruction {
     return hasFnAttrOnCalledFunction(Kind);
   }
 
+  /// A specialized version of hasFnAttrImpl for when the caller wants to
+  /// know if an attribute's semantics are implied, not whether the attribute
+  /// is actually present.  This distinction only exists when checking whether
+  /// something is readonly or writeonly since readnone implies both.  The case
+  /// which motivates the specialized code is a callee with readnone, and an
+  /// operand bundle on the call which disallows readnone but not either
+  /// readonly or writeonly.
+  bool hasImpliedFnAttr(Attribute::AttrKind Kind) const {
+    assert((Kind == Attribute::ReadOnly || Kind == Attribute::WriteOnly) &&
+           "use hasFnAttrImpl instead");
+    if (Attrs.hasFnAttr(Kind) || Attrs.hasFnAttr(Attribute::ReadNone))
+      return true;
+
+    if (isFnAttrDisallowedByOpBundle(Kind))
+      return false;
+
+    return hasFnAttrOnCalledFunction(Kind) ||
+      hasFnAttrOnCalledFunction(Attribute::ReadNone);
+  }
+
   /// Determine whether the return value has the given attribute. Supports
   /// Attribute::AttrKind and StringRef as \p AttrKind types.
   template <typename AttrKind> bool hasRetAttrImpl(AttrKind Kind) const {

diff  --git a/llvm/test/Transforms/InstCombine/trivial-dse-calls.ll b/llvm/test/Transforms/InstCombine/trivial-dse-calls.ll
index 5e89789a0e23d..29755575ddd98 100644
--- a/llvm/test/Transforms/InstCombine/trivial-dse-calls.ll
+++ b/llvm/test/Transforms/InstCombine/trivial-dse-calls.ll
@@ -253,7 +253,6 @@ define void @test_readnone() {
 
 define void @test_readnone_with_deopt() {
 ; CHECK-LABEL: @test_readnone_with_deopt(
-; CHECK-NEXT:    call void @removable_readnone() [ "deopt"() ]
 ; CHECK-NEXT:    ret void
 ;
   call void @removable_readnone() [ "deopt"() ]