[PATCH] D55638: [CallSite removal] Add and flesh out APIs on the new `CallBase` base class that previously were only available on the `CallSite` wrapper.

Saleem Abdulrasool via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 27 08:30:16 PST 2018


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

The only minor nit is the preference for `Known` in some of the operand checks.  IIRC clang doesn't get 100% of the annotations, so even if something is not marked as `noread`, it may be `noread`.  Yes, its nit-picky, but, I think it helps when dealing with code that you are unfamiliar with (or happen to page out the context from clang).



================
Comment at: llvm/include/llvm/IR/InstrTypes.h:1388
+  /// Determine whether this data operand is not captured.
+  bool doesNotCapture(unsigned OpNo) const {
+    return dataOperandHasImpliedAttr(OpNo + 1, Attribute::NoCapture);
----------------
Can we name this `isKnownNonCapture`?  The attributes aren't guaranteed to be emitted by the frontend.


================
Comment at: llvm/include/llvm/IR/InstrTypes.h:1393
+  /// Determine whether this argument is passed by value.
+  bool isByValArgument(unsigned ArgNo) const {
+    return paramHasAttr(ArgNo, Attribute::ByVal);
----------------
Can we name this `isKnownByValArgument`?


================
Comment at: llvm/include/llvm/IR/InstrTypes.h:1414
+
+  bool doesNotAccessMemory(unsigned OpNo) const {
+    return dataOperandHasImpliedAttr(OpNo + 1, Attribute::ReadNone);
----------------
Can we name this with `Known` in the name as well?


================
Comment at: llvm/include/llvm/IR/InstrTypes.h:1418
+
+  bool onlyReadsMemory(unsigned OpNo) const {
+    return dataOperandHasImpliedAttr(OpNo + 1, Attribute::ReadOnly) ||
----------------
Similar


================
Comment at: llvm/include/llvm/IR/InstrTypes.h:1423
+
+  bool doesNotReadMemory(unsigned OpNo) const {
+    return dataOperandHasImpliedAttr(OpNo + 1, Attribute::WriteOnly) ||
----------------
Similar


================
Comment at: llvm/lib/IR/Instructions.cpp:269
+    return true;
+  else if (getDereferenceableBytes(AttributeList::ReturnIndex) > 0 &&
+           !NullPointerIsDefined(getCaller(),
----------------
Unnecessary `else`.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55638





More information about the llvm-commits mailing list