[llvm] 0fd018f - [NFC] [AAPointerInfo] OffsetAndSize is no longer an std::pair
Sameer Sahasrabuddhe via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 26 22:32:25 PDT 2022
Author: Sameer Sahasrabuddhe
Date: 2022-10-27T11:00:28+05:30
New Revision: 0fd018f9a99fd51567dabf570e9a379f1ab15ccd
URL: https://github.com/llvm/llvm-project/commit/0fd018f9a99fd51567dabf570e9a379f1ab15ccd
DIFF: https://github.com/llvm/llvm-project/commit/0fd018f9a99fd51567dabf570e9a379f1ab15ccd.diff
LOG: [NFC] [AAPointerInfo] OffsetAndSize is no longer an std::pair
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
Added:
Modified:
llvm/include/llvm/Transforms/IPO/Attributor.h
llvm/lib/Transforms/IPO/Attributor.cpp
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Transforms/IPO/Attributor.h b/llvm/include/llvm/Transforms/IPO/Attributor.h
index d6cdad242ced0..61c26dfabed0b 100644
--- a/llvm/include/llvm/Transforms/IPO/Attributor.h
+++ b/llvm/include/llvm/Transforms/IPO/Attributor.h
@@ -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,
diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index a48f956f943f6..f7024f441a08c 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -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,
diff --git a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
index 2a3ca28b97c46..76025a99893af 100644
--- a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
+++ b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
@@ -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 @@ struct AA::PointerInfo::State : public AbstractState {
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 @@ struct AA::PointerInfo::State : public AbstractState {
break;
}
}
- if (OAS.getSize() != AA::OffsetAndSize::Unassigned)
+ if (OAS.Size != AA::OffsetAndSize::Unassigned)
break;
}
@@ -930,7 +948,7 @@ struct AA::PointerInfo::State : public AbstractState {
*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";
More information about the llvm-commits
mailing list