[llvm] 4136897 - [DebugInfo][InstrRef][NFC] Switch to using DenseMaps and similar
Jeremy Morse via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 25 10:08:12 PDT 2021
Author: Jeremy Morse
Date: 2021-10-25T18:07:17+01:00
New Revision: 4136897bd4541f5c5e9677e297dabcfe9e367010
URL: https://github.com/llvm/llvm-project/commit/4136897bd4541f5c5e9677e297dabcfe9e367010
DIFF: https://github.com/llvm/llvm-project/commit/4136897bd4541f5c5e9677e297dabcfe9e367010.diff
LOG: [DebugInfo][InstrRef][NFC] Switch to using DenseMaps and similar
There are a few STL containers hanging around that can become DenseMaps,
SmallVectors and similar. This recovers a modest amount of compile time
performance.
While I'm here, adjust the bit layout of ValueIDNum: this was always
supposed to act like a value type, however it seems that clang doesn't
compile the comparison functions to act that way. Add a uint64_t to a
union that explicitly aliases the bitfields, so that we can compare the
whole value as a single integer.
Differential Revision: https://reviews.llvm.org/D112333
Added:
Modified:
llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h
llvm/unittests/CodeGen/InstrRefLDVTest.cpp
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
index ccb5b2f3118b..231367ed2003 100644
--- a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
+++ b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
@@ -196,12 +196,12 @@ class TransferTracker {
/// between TransferTrackers view of variable locations and MLocTrackers. For
/// example, MLocTracker observes all clobbers, but TransferTracker lazily
/// does not.
- std::vector<ValueIDNum> VarLocs;
+ SmallVector<ValueIDNum, 32> VarLocs;
/// Map from LocIdxes to which DebugVariables are based that location.
/// Mantained while stepping through the block. Not accurate if
/// VarLocs[Idx] != MTracker->LocIdxToIDNum[Idx].
- std::map<LocIdx, SmallSet<DebugVariable, 4>> ActiveMLocs;
+ DenseMap<LocIdx, SmallSet<DebugVariable, 4>> ActiveMLocs;
/// Map from DebugVariable to it's current location and qualifying meta
/// information. To be used in conjunction with ActiveMLocs to construct
@@ -273,6 +273,8 @@ class TransferTracker {
// Map of the preferred location for each value.
std::map<ValueIDNum, LocIdx> ValueToLoc;
+ ActiveMLocs.reserve(VLocs.size());
+ ActiveVLocs.reserve(VLocs.size());
// Produce a map of value numbers to the current machine locs they live
// in. When emulating VarLocBasedImpl, there should only be one
@@ -568,6 +570,8 @@ class TransferTracker {
flushDbgValues(Pos, nullptr);
+ // Re-find ActiveMLocIt, iterator could have been invalidated.
+ ActiveMLocIt = ActiveMLocs.find(MLoc);
ActiveMLocIt->second.clear();
}
@@ -582,11 +586,14 @@ class TransferTracker {
// assert(ActiveMLocs[Dst].size() == 0);
//^^^ Legitimate scenario on account of un-clobbered slot being assigned to?
- ActiveMLocs[Dst] = ActiveMLocs[Src];
+
+ // Move set of active variables from one location to another.
+ auto MovingVars = ActiveMLocs[Src];
+ ActiveMLocs[Dst] = MovingVars;
VarLocs[Dst.asU64()] = VarLocs[Src.asU64()];
// For each variable based on Src; create a location at Dst.
- for (auto &Var : ActiveMLocs[Src]) {
+ for (auto &Var : MovingVars) {
auto ActiveVLocIt = ActiveVLocs.find(Var);
assert(ActiveVLocIt != ActiveVLocs.end());
ActiveVLocIt->second.Loc = Dst;
@@ -627,6 +634,7 @@ class TransferTracker {
//===----------------------------------------------------------------------===//
ValueIDNum ValueIDNum::EmptyValue = {UINT_MAX, UINT_MAX, UINT_MAX};
+ValueIDNum ValueIDNum::TombstoneValue = {UINT_MAX, UINT_MAX, UINT_MAX - 1};
#ifndef NDEBUG
void DbgValue::dump(const MLocTracker *MTrack) const {
diff --git a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h
index 21bbe90bf7b6..d96ef6d4f6e5 100644
--- a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h
+++ b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h
@@ -55,6 +55,11 @@ class LocIdx {
}
static LocIdx MakeIllegalLoc() { return LocIdx(); }
+ static LocIdx MakeTombstoneLoc() {
+ LocIdx L = LocIdx();
+ --L.Location;
+ return L;
+ }
bool isIllegal() const { return Location == UINT_MAX; }
@@ -101,36 +106,43 @@ struct SpillLoc {
/// problematic; but by that point we should probably have bailed out of
/// trying to analyse the function.
class ValueIDNum {
- uint64_t BlockNo : 20; /// The block where the def happens.
- uint64_t InstNo : 20; /// The Instruction where the def happens.
- /// One based, is distance from start of block.
- uint64_t LocNo : NUM_LOC_BITS; /// The machine location where the def happens.
+ union {
+ struct {
+ uint64_t BlockNo : 20; /// The block where the def happens.
+ uint64_t InstNo : 20; /// The Instruction where the def happens.
+ /// One based, is distance from start of block.
+ uint64_t LocNo
+ : NUM_LOC_BITS; /// The machine location where the def happens.
+ } s;
+ uint64_t Value;
+ } u;
+
+ static_assert(sizeof(u) == 8, "Badly packed ValueIDNum?");
public:
// Default-initialize to EmptyValue. This is necessary to make IndexedMaps
// of values to work.
- ValueIDNum() : BlockNo(0xFFFFF), InstNo(0xFFFFF), LocNo(0xFFFFFF) {}
+ ValueIDNum() { u.Value = EmptyValue.asU64(); }
- ValueIDNum(uint64_t Block, uint64_t Inst, uint64_t Loc)
- : BlockNo(Block), InstNo(Inst), LocNo(Loc) {}
+ ValueIDNum(uint64_t Block, uint64_t Inst, uint64_t Loc) {
+ u.s = {Block, Inst, Loc};
+ }
- ValueIDNum(uint64_t Block, uint64_t Inst, LocIdx Loc)
- : BlockNo(Block), InstNo(Inst), LocNo(Loc.asU64()) {}
+ ValueIDNum(uint64_t Block, uint64_t Inst, LocIdx Loc) {
+ u.s = {Block, Inst, Loc.asU64()};
+ }
- uint64_t getBlock() const { return BlockNo; }
- uint64_t getInst() const { return InstNo; }
- uint64_t getLoc() const { return LocNo; }
- bool isPHI() const { return InstNo == 0; }
+ uint64_t getBlock() const { return u.s.BlockNo; }
+ uint64_t getInst() const { return u.s.InstNo; }
+ uint64_t getLoc() const { return u.s.LocNo; }
+ bool isPHI() const { return u.s.InstNo == 0; }
- uint64_t asU64() const {
- uint64_t TmpBlock = BlockNo;
- uint64_t TmpInst = InstNo;
- return TmpBlock << 44ull | TmpInst << NUM_LOC_BITS | LocNo;
- }
+ uint64_t asU64() const { return u.Value; }
static ValueIDNum fromU64(uint64_t v) {
- uint64_t L = (v & 0x3FFF);
- return {v >> 44ull, ((v >> NUM_LOC_BITS) & 0xFFFFF), L};
+ ValueIDNum Val;
+ Val.u.Value = v;
+ return Val;
}
bool operator<(const ValueIDNum &Other) const {
@@ -138,23 +150,25 @@ class ValueIDNum {
}
bool operator==(const ValueIDNum &Other) const {
- return std::tie(BlockNo, InstNo, LocNo) ==
- std::tie(Other.BlockNo, Other.InstNo, Other.LocNo);
+ return u.Value == Other.u.Value;
}
bool operator!=(const ValueIDNum &Other) const { return !(*this == Other); }
std::string asString(const std::string &mlocname) const {
return Twine("Value{bb: ")
- .concat(Twine(BlockNo).concat(
- Twine(", inst: ")
- .concat((InstNo ? Twine(InstNo) : Twine("live-in"))
- .concat(Twine(", loc: ").concat(Twine(mlocname)))
- .concat(Twine("}")))))
+ .concat(Twine(u.s.BlockNo)
+ .concat(Twine(", inst: ")
+ .concat((u.s.InstNo ? Twine(u.s.InstNo)
+ : Twine("live-in"))
+ .concat(Twine(", loc: ").concat(
+ Twine(mlocname)))
+ .concat(Twine("}")))))
.str();
}
static ValueIDNum EmptyValue;
+ static ValueIDNum TombstoneValue;
};
/// Thin wrapper around an integer -- designed to give more type safety to
@@ -716,7 +730,7 @@ class InstrRefBasedLDV : public LDVImpl {
/// Machine location/value transfer function, a mapping of which locations
/// are assigned which new values.
- using MLocTransferMap = std::map<LocIdx, ValueIDNum>;
+ using MLocTransferMap = SmallDenseMap<LocIdx, ValueIDNum>;
/// Live in/out structure for the variable values: a per-block map of
/// variables to their values.
@@ -1007,4 +1021,31 @@ class InstrRefBasedLDV : public LDVImpl {
} // namespace LiveDebugValues
+namespace llvm {
+using namespace LiveDebugValues;
+
+template <> struct DenseMapInfo<LocIdx> {
+ static inline LocIdx getEmptyKey() { return LocIdx::MakeIllegalLoc(); }
+ static inline LocIdx getTombstoneKey() { return LocIdx::MakeTombstoneLoc(); }
+
+ static unsigned getHashValue(const LocIdx &Loc) { return Loc.asU64(); }
+
+ static bool isEqual(const LocIdx &A, const LocIdx &B) { return A == B; }
+};
+
+template <> struct DenseMapInfo<ValueIDNum> {
+ static inline ValueIDNum getEmptyKey() { return ValueIDNum::EmptyValue; }
+ static inline ValueIDNum getTombstoneKey() {
+ return ValueIDNum::TombstoneValue;
+ }
+
+ static unsigned getHashValue(const ValueIDNum &Val) { return Val.asU64(); }
+
+ static bool isEqual(const ValueIDNum &A, const ValueIDNum &B) {
+ return A == B;
+ }
+};
+
+} // end namespace llvm
+
#endif /* LLVM_LIB_CODEGEN_LIVEDEBUGVALUES_INSTRREFBASEDLDV_H */
diff --git a/llvm/unittests/CodeGen/InstrRefLDVTest.cpp b/llvm/unittests/CodeGen/InstrRefLDVTest.cpp
index ac74e8a5cf0b..29a98f960abe 100644
--- a/llvm/unittests/CodeGen/InstrRefLDVTest.cpp
+++ b/llvm/unittests/CodeGen/InstrRefLDVTest.cpp
@@ -903,7 +903,8 @@ TEST_F(InstrRefLDVTest, MLocSingleBlock) {
ValueIDNum *OutLocsPtr[1] = {&OutLocs[0]};
// Transfer function: nothing.
- SmallVector<MLocTransferMap, 1> TransferFunc = {{}};
+ SmallVector<MLocTransferMap, 1> TransferFunc;
+ TransferFunc.resize(1);
// Try and build value maps...
buildMLocValueMap(InLocsPtr, OutLocsPtr, TransferFunc);
More information about the llvm-commits
mailing list