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

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 18 12:18:16 PST 2022


fhahn accepted this revision.
fhahn added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!



================
Comment at: llvm/include/llvm/IR/InstrTypes.h:2291
 
+  // A specialized version of hasFnAttrImpl for when the caller wants to
+  // know if an attribute's semantics are implied, not whether the attribute
----------------
nit: doc-comment should use `///`?


================
Comment at: llvm/include/llvm/IR/InstrTypes.h:2293
+  // 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
----------------
Nit: 2 spaces before `This`.


================
Comment at: llvm/include/llvm/IR/InstrTypes.h:2294
+  // 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
----------------
nit: 2 spaces before `The`.


================
Comment at: llvm/test/Transforms/InstCombine/trivial-dse-calls.ll:258
 ;
   call void @removable_readnone() [ "deopt"() ]
   ret void
----------------
Do we also have a test with an unknown bundle?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117591



More information about the llvm-commits mailing list