[PATCH] D117591: A readonly operand bundle should not prevent inference of readonly from a readnone callee

[Philip Reames via Phabricator via llvm-commits]

reames created this revision.
reames added reviewers: nikic, apilipenko, fhahn, skatkov, annatom.
Herald added subscribers: dexonsmith, bollu, mcrosier.
reames requested review of this revision.
Herald added a project: LLVM.

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.

Review note: The change for writeonly is an unrelated bug fix, and I'll be landing it separately and rebasing.  Included it here just because the cycle time on building changes to such a key header are painful.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117591

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


Index: llvm/test/Transforms/InstCombine/trivial-dse-calls.ll
===================================================================
--- llvm/test/Transforms/InstCombine/trivial-dse-calls.ll
+++ llvm/test/Transforms/InstCombine/trivial-dse-calls.ll
@@ -253,7 +253,6 @@
 
 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"() ]
Index: llvm/include/llvm/IR/InstrTypes.h
===================================================================
--- llvm/include/llvm/IR/InstrTypes.h
+++ llvm/include/llvm/IR/InstrTypes.h
@@ -1818,14 +1818,14 @@
 
   /// 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); }
 
@@ -2113,6 +2113,9 @@
 
     case Attribute::ReadOnly:
       return hasClobberingOperandBundles();
+
+    case Attribute::WriteOnly:
+      return hasReadingOperandBundles();
     }
 
     llvm_unreachable("switch has a default case!");
@@ -2285,6 +2288,26 @@
     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 {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D117591.400920.patch
Type: text/x-patch
Size: 2767 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220118/f78b55f1/attachment.bin>