[NFC] [AAPointerInfo] OffsetAndSize is no longer an std::pair
authorSameer Sahasrabuddhe <sameer.sahasrabuddhe@amd.com>
Thu, 27 Oct 2022 05:15:29 +0000 (10:45 +0530)
committerSameer Sahasrabuddhe <sameer.sahasrabuddhe@amd.com>
Thu, 27 Oct 2022 05:30:28 +0000 (11:00 +0530)
The struct OffsetAndSize is a simple tuple of two int64_t. Treating it as a
derived class of std::pair has no special benefit, but it makes the code
verbose since we need get/set functions that avoid using "first" and "second" in
client code. Eliminating the std::pair makes this more readable.

Reviewed By: jdoerfert

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

llvm/include/llvm/Transforms/IPO/Attributor.h
llvm/lib/Transforms/IPO/Attributor.cpp
llvm/lib/Transforms/IPO/AttributorAttributes.cpp

index d6cdad2..61c26df 100644 (file)
@@ -211,36 +211,31 @@ combineOptionalValuesInAAValueLatice(const Optional<Value *> &A,
 
 /// Helper to represent an access offset and size, with logic to deal with
 /// uncertainty and check for overlapping accesses.
-struct OffsetAndSize : public std::pair<int64_t, int64_t> {
-  using BaseTy = std::pair<int64_t, int64_t>;
-  OffsetAndSize(int64_t Offset, int64_t Size) : BaseTy(Offset, Size) {}
-  OffsetAndSize(const BaseTy &P) : BaseTy(P) {}
-  int64_t getOffset() const { return first; }
-  int64_t getSize() const { return second; }
-  static OffsetAndSize getUnknown() { return OffsetAndSize(Unknown, Unknown); }
-  static OffsetAndSize getUnassigned() {
-    return OffsetAndSize(Unassigned, Unassigned);
-  }
+struct OffsetAndSize {
+  int64_t Offset = Unassigned;
+  int64_t Size = Unassigned;
+
+  OffsetAndSize(int64_t Offset, int64_t Size) : Offset(Offset), Size(Size) {}
+  OffsetAndSize() = default;
+  static OffsetAndSize getUnknown() { return OffsetAndSize{Unknown, Unknown}; }
 
   /// Return true if offset or size are unknown.
   bool offsetOrSizeAreUnknown() const {
-    return getOffset() == OffsetAndSize::Unknown ||
-           getSize() == OffsetAndSize::Unknown;
+    return Offset == OffsetAndSize::Unknown || Size == OffsetAndSize::Unknown;
   }
 
   /// Return true if offset and size are unknown, thus this is the default
   /// unknown object.
   bool offsetAndSizeAreUnknown() const {
-    return getOffset() == OffsetAndSize::Unknown &&
-           getSize() == OffsetAndSize::Unknown;
+    return Offset == OffsetAndSize::Unknown && Size == OffsetAndSize::Unknown;
   }
 
   /// Return true if the offset and size are unassigned.
   bool isUnassigned() const {
-    assert((getOffset() == OffsetAndSize::Unassigned) ==
-               (getSize() == OffsetAndSize::Unassigned) &&
+    assert((Offset == OffsetAndSize::Unassigned) ==
+               (Size == OffsetAndSize::Unassigned) &&
            "Inconsistent state!");
-    return getOffset() == OffsetAndSize::Unassigned;
+    return Offset == OffsetAndSize::Unassigned;
   }
 
   /// Return true if this offset and size pair might describe an address that
@@ -252,15 +247,25 @@ struct OffsetAndSize : public std::pair<int64_t, int64_t> {
 
     // Check if one offset point is in the other interval [offset,
     // offset+size].
-    return OAS.getOffset() + OAS.getSize() > getOffset() &&
-           OAS.getOffset() < getOffset() + getSize();
+    return OAS.Offset + OAS.Size > Offset && OAS.Offset < Offset + Size;
   }
 
   /// Constants used to represent special offsets or sizes.
+  /// - This assumes that Offset and Size are non-negative.
+  /// - The constants should not clash with DenseMapInfo, such as EmptyKey
+  ///   (INT64_MAX) and TombstoneKey (INT64_MIN).
   static constexpr int64_t Unassigned = -1;
   static constexpr int64_t Unknown = -2;
 };
 
+inline bool operator==(const OffsetAndSize &A, const OffsetAndSize &B) {
+  return A.Offset == B.Offset && A.Size == B.Size;
+}
+
+inline bool operator!=(const OffsetAndSize &A, const OffsetAndSize &B) {
+  return !(A == B);
+}
+
 /// Return the initial value of \p Obj with type \p Ty if that is a constant.
 Constant *getInitialValueForObj(Value &Obj, Type &Ty,
                                 const TargetLibraryInfo *TLI,
index a48f956..f7024f4 100644 (file)
@@ -238,7 +238,7 @@ Constant *AA::getInitialValueForObj(Value &Obj, Type &Ty,
     return UndefValue::get(&Ty);
 
   if (OASPtr && !OASPtr->offsetOrSizeAreUnknown()) {
-    APInt Offset = APInt(64, OASPtr->getOffset());
+    APInt Offset = APInt(64, OASPtr->Offset);
     return ConstantFoldLoadFromConst(GV->getInitializer(), &Ty, Offset, DL);
   }
 
@@ -452,7 +452,7 @@ static bool getPotentialCopiesOfMemoryValue(
     // object.
     bool HasBeenWrittenTo = false;
 
-    AA::OffsetAndSize OAS = AA::OffsetAndSize::getUnassigned();
+    AA::OffsetAndSize OAS;
     auto &PI = A.getAAFor<AAPointerInfo>(QueryingAA, IRPosition::value(*Obj),
                                          DepClassTy::NONE);
     if (!PI.forallInterferingAccesses(A, QueryingAA, I, CheckAccess,
index 2a3ca28..76025a9 100644 (file)
@@ -720,9 +720,27 @@ struct DenseMapInfo<AAPointerInfo::Access> : DenseMapInfo<Instruction *> {
 };
 
 /// Helper that allows OffsetAndSize as a key in a DenseMap.
-template <>
-struct DenseMapInfo<AA::OffsetAndSize>
-    : DenseMapInfo<std::pair<int64_t, int64_t>> {};
+template <> struct DenseMapInfo<AA::OffsetAndSize> {
+  static inline AA::OffsetAndSize getEmptyKey() {
+    auto EmptyKey = DenseMapInfo<int64_t>::getEmptyKey();
+    return AA::OffsetAndSize{EmptyKey, EmptyKey};
+  }
+
+  static inline AA::OffsetAndSize getTombstoneKey() {
+    auto TombstoneKey = DenseMapInfo<int64_t>::getTombstoneKey();
+    return AA::OffsetAndSize{TombstoneKey, TombstoneKey};
+  }
+
+  static unsigned getHashValue(const AA::OffsetAndSize &OAS) {
+    return detail::combineHashValue(
+        DenseMapInfo<int64_t>::getHashValue(OAS.Offset),
+        DenseMapInfo<int64_t>::getHashValue(OAS.Size));
+  }
+
+  static bool isEqual(const AA::OffsetAndSize &A, const AA::OffsetAndSize B) {
+    return A == B;
+  }
+};
 
 /// Helper for AA::PointerInfo::Access DenseMap/Set usage ignoring everythign
 /// but the instruction
@@ -914,7 +932,7 @@ protected:
       return false;
 
     // First find the offset and size of I.
-    AA::OffsetAndSize OAS = AA::OffsetAndSize::getUnassigned();
+    AA::OffsetAndSize OAS;
     for (const auto &It : AccessBins) {
       for (auto &Access : *It.getSecond()) {
         if (Access.getRemoteInst() == &I) {
@@ -922,7 +940,7 @@ protected:
           break;
         }
       }
-      if (OAS.getSize() != AA::OffsetAndSize::Unassigned)
+      if (OAS.Size != AA::OffsetAndSize::Unassigned)
         break;
     }
 
@@ -930,7 +948,7 @@ protected:
       *OASPtr = OAS;
 
     // No access for I was found, we are done.
-    if (OAS.getSize() == AA::OffsetAndSize::Unassigned)
+    if (OAS.Size == AA::OffsetAndSize::Unassigned)
       return true;
 
     // Now that we have an offset and size, find all overlapping ones and use
@@ -1157,8 +1175,7 @@ struct AAPointerInfoImpl
     for (const auto &It : OtherAAImpl.getState()) {
       AA::OffsetAndSize OAS = AA::OffsetAndSize::getUnknown();
       if (Offset != AA::OffsetAndSize::Unknown)
-        OAS = AA::OffsetAndSize(It.first.getOffset() + Offset,
-                                It.first.getSize());
+        OAS = AA::OffsetAndSize(It.first.Offset + Offset, It.first.Size);
       Accesses *Bin = AccessBins.lookup(OAS);
       for (const AAPointerInfo::Access &RAcc : *It.second) {
         if (IsByval && !RAcc.isRead())
@@ -1174,8 +1191,8 @@ struct AAPointerInfoImpl
           AK = AccessKind(AK | (RAcc.isMayAccess() ? AK_MAY : AK_MUST));
         }
         Changed =
-            Changed | addAccess(A, OAS.getOffset(), OAS.getSize(), CB, Content,
-                                AK, RAcc.getType(), RAcc.getRemoteInst(), Bin);
+            Changed | addAccess(A, OAS.Offset, OAS.Size, CB, Content, AK,
+                                RAcc.getType(), RAcc.getRemoteInst(), Bin);
       }
     }
     return Changed;
@@ -1188,8 +1205,7 @@ struct AAPointerInfoImpl
   /// Dump the state into \p O.
   void dumpState(raw_ostream &O) {
     for (auto &It : AccessBins) {
-      O << "[" << It.first.getOffset() << "-"
-        << It.first.getOffset() + It.first.getSize()
+      O << "[" << It.first.Offset << "-" << It.first.Offset + It.first.Size
         << "] : " << It.getSecond()->size() << "\n";
       for (auto &Acc : *It.getSecond()) {
         O << "     - " << Acc.getKind() << " - " << *Acc.getLocalInst() << "\n";