[PATCH] D109390: [NFC] Cleanup off by one indexes in CallBase::dataOperandHasImpliedAttr()

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks created this revision.
aeubanks added a reviewer: rnk.
Herald added subscribers: dexonsmith, arphaman.
aeubanks requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Verified that previously nothing was calling dataOperandHasImpliedAttr()
with AttributeList::ReturnIndex even though we had a code path for it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109390

Files:
  llvm/include/llvm/IR/InstrTypes.h


Index: llvm/include/llvm/IR/InstrTypes.h
===================================================================
--- llvm/include/llvm/IR/InstrTypes.h
+++ llvm/include/llvm/IR/InstrTypes.h
@@ -1631,42 +1631,35 @@
   /// A.
   ///
   /// Data operands include call arguments and values used in operand bundles,
-  /// but does not include the callee operand.  This routine dispatches to the
-  /// underlying AttributeList or the OperandBundleUser as appropriate.
+  /// but does not include the callee operand.
   ///
   /// The index \p i is interpreted as
   ///
-  ///  \p i == Attribute::ReturnIndex  -> the return value
-  ///  \p i in [1, arg_size + 1)  -> argument number (\p i - 1)
-  ///  \p i in [arg_size + 1, data_operand_size + 1) -> bundle operand at index
-  ///     (\p i - 1) in the operand list.
+  ///  \p i in [0, arg_size)  -> argument number (\p i)
+  ///  \p i in [arg_size, data_operand_size) -> bundle operand at index
+  ///     (\p i) in the operand list.
   bool dataOperandHasImpliedAttr(unsigned i, Attribute::AttrKind Kind) const {
     // Note that we have to add one because `i` isn't zero-indexed.
-    assert(i < (getNumArgOperands() + getNumTotalBundleOperands() + 1) &&
+    assert(i < getNumArgOperands() + getNumTotalBundleOperands() &&
            "Data operand index out of bounds!");
 
     // The attribute A can either be directly specified, if the operand in
     // question is a call argument; or be indirectly implied by the kind of its
     // containing operand bundle, if the operand is a bundle operand.
 
-    if (i == AttributeList::ReturnIndex)
-      return hasRetAttr(Kind);
+    if (i < getNumArgOperands())
+      return paramHasAttr(i, Kind);
 
-    // FIXME: Avoid these i - 1 calculations and update the API to use
-    // zero-based indices.
-    if (i < (getNumArgOperands() + 1))
-      return paramHasAttr(i - 1, Kind);
-
-    assert(hasOperandBundles() && i >= (getBundleOperandsStartIndex() + 1) &&
+    assert(hasOperandBundles() && i >= getBundleOperandsStartIndex() &&
            "Must be either a call argument or an operand bundle!");
-    return bundleOperandHasAttr(i - 1, Kind);
+    return bundleOperandHasAttr(i, Kind);
   }
 
   /// Determine whether this data operand is not captured.
   // FIXME: Once this API is no longer duplicated in `CallSite`, rename this to
   // better indicate that this may return a conservative answer.
   bool doesNotCapture(unsigned OpNo) const {
-    return dataOperandHasImpliedAttr(OpNo + 1, Attribute::NoCapture);
+    return dataOperandHasImpliedAttr(OpNo, Attribute::NoCapture);
   }
 
   /// Determine whether this argument is passed by value.
@@ -1707,21 +1700,21 @@
   // FIXME: Once this API is no longer duplicated in `CallSite`, rename this to
   // better indicate that this may return a conservative answer.
   bool doesNotAccessMemory(unsigned OpNo) const {
-    return dataOperandHasImpliedAttr(OpNo + 1, Attribute::ReadNone);
+    return dataOperandHasImpliedAttr(OpNo, Attribute::ReadNone);
   }
 
   // FIXME: Once this API is no longer duplicated in `CallSite`, rename this to
   // better indicate that this may return a conservative answer.
   bool onlyReadsMemory(unsigned OpNo) const {
-    return dataOperandHasImpliedAttr(OpNo + 1, Attribute::ReadOnly) ||
-           dataOperandHasImpliedAttr(OpNo + 1, Attribute::ReadNone);
+    return dataOperandHasImpliedAttr(OpNo, Attribute::ReadOnly) ||
+           dataOperandHasImpliedAttr(OpNo, Attribute::ReadNone);
   }
 
   // FIXME: Once this API is no longer duplicated in `CallSite`, rename this to
   // better indicate that this may return a conservative answer.
   bool doesNotReadMemory(unsigned OpNo) const {
-    return dataOperandHasImpliedAttr(OpNo + 1, Attribute::WriteOnly) ||
-           dataOperandHasImpliedAttr(OpNo + 1, Attribute::ReadNone);
+    return dataOperandHasImpliedAttr(OpNo, Attribute::WriteOnly) ||
+           dataOperandHasImpliedAttr(OpNo, Attribute::ReadNone);
   }
 
   /// Extract the alignment of the return value.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D109390.371168.patch
Type: text/x-patch
Size: 4032 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210907/9db545f5/attachment.bin>