Skip to content

[ADT] Overhaul the DenseMapIterator creation logic (NFC) #156221

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 1, 2025
+49 −57
Merged

[ADT] Overhaul the DenseMapIterator creation logic (NFC) #156221

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 49 additions & 57 deletions llvm/include/llvm/ADT/DenseMap.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,26 +76,14 @@ class DenseMapBase : public DebugEpochBase {
DenseMapIterator<KeyT, ValueT, KeyInfoT, BucketT, true>;

inline iterator begin() {
// When the map is empty, avoid the overhead of advancing/retreating past
// empty buckets.
if (empty())
return end();
if (shouldReverseIterate<KeyT>())
return makeIterator(getBucketsEnd() - 1, getBuckets(), *this);
return makeIterator(getBuckets(), getBucketsEnd(), *this);
}
inline iterator end() {
return makeIterator(getBucketsEnd(), getBucketsEnd(), *this, true);
return iterator::makeBegin(buckets(), empty(), *this);
}
inline iterator end() { return iterator::makeEnd(buckets(), *this); }
inline const_iterator begin() const {
if (empty())
return end();
if (shouldReverseIterate<KeyT>())
return makeConstIterator(getBucketsEnd() - 1, getBuckets(), *this);
return makeConstIterator(getBuckets(), getBucketsEnd(), *this);
return const_iterator::makeBegin(buckets(), empty(), *this);
}
inline const_iterator end() const {
return makeConstIterator(getBucketsEnd(), getBucketsEnd(), *this, true);
return const_iterator::makeEnd(buckets(), *this);
}

// Return an iterator to iterate over keys in the map.
Expand Down Expand Up @@ -184,17 +172,13 @@ class DenseMapBase : public DebugEpochBase {
/// type used.
template <class LookupKeyT> iterator find_as(const LookupKeyT &Val) {
if (BucketT *Bucket = doFind(Val))
return makeIterator(
Bucket, shouldReverseIterate<KeyT>() ? getBuckets() : getBucketsEnd(),
*this, true);
return makeIterator(Bucket);
return end();
}
template <class LookupKeyT>
const_iterator find_as(const LookupKeyT &Val) const {
if (const BucketT *Bucket = doFind(Val))
return makeConstIterator(
Bucket, shouldReverseIterate<KeyT>() ? getBuckets() : getBucketsEnd(),
*this, true);
return makeConstIterator(Bucket);
return end();
}

Expand Down Expand Up @@ -264,13 +248,13 @@ class DenseMapBase : public DebugEpochBase {
const LookupKeyT &Val) {
BucketT *TheBucket;
if (LookupBucketFor(Val, TheBucket))
return {makeInsertIterator(TheBucket), false}; // Already in map.
return {makeIterator(TheBucket), false}; // Already in map.

// Otherwise, insert the new element.
TheBucket = findBucketForInsertion(Val, TheBucket);
TheBucket->getFirst() = std::move(KV.first);
::new (&TheBucket->getSecond()) ValueT(std::move(KV.second));
return {makeInsertIterator(TheBucket), true};
return {makeIterator(TheBucket), true};
}

/// insert - Range insertion of pairs.
Expand Down Expand Up @@ -482,33 +466,15 @@ class DenseMapBase : public DebugEpochBase {
std::pair<iterator, bool> try_emplace_impl(KeyArgT &&Key, Ts &&...Args) {
auto [Bucket, Inserted] = lookupOrInsertIntoBucket(
std::forward<KeyArgT>(Key), std::forward<Ts>(Args)...);
return {makeInsertIterator(Bucket), Inserted};
return {makeIterator(Bucket), Inserted};
}

iterator makeIterator(BucketT *P, BucketT *E, DebugEpochBase &Epoch,
bool NoAdvance = false) {
if (shouldReverseIterate<KeyT>()) {
BucketT *B = P == getBucketsEnd() ? getBuckets() : P + 1;
return iterator(B, E, Epoch, NoAdvance);
}
return iterator(P, E, Epoch, NoAdvance);
iterator makeIterator(BucketT *TheBucket) {
return iterator::makeIterator(TheBucket, buckets(), *this);
}

const_iterator makeConstIterator(const BucketT *P, const BucketT *E,
const DebugEpochBase &Epoch,
const bool NoAdvance = false) const {
if (shouldReverseIterate<KeyT>()) {
const BucketT *B = P == getBucketsEnd() ? getBuckets() : P + 1;
return const_iterator(B, E, Epoch, NoAdvance);
}
return const_iterator(P, E, Epoch, NoAdvance);
}

iterator makeInsertIterator(BucketT *TheBucket) {
return makeIterator(TheBucket,
shouldReverseIterate<KeyT>() ? getBuckets()
: getBucketsEnd(),
*this, true);
const_iterator makeConstIterator(const BucketT *TheBucket) const {
return const_iterator::makeIterator(TheBucket, buckets(), *this);
}

unsigned getNumEntries() const {
Expand Down Expand Up @@ -555,6 +521,10 @@ class DenseMapBase : public DebugEpochBase {
return llvm::make_range(getBuckets(), getBucketsEnd());
}

iterator_range<const BucketT *> buckets() const {
return llvm::make_range(getBuckets(), getBucketsEnd());
}

void grow(unsigned AtLeast) { static_cast<DerivedT *>(this)->grow(AtLeast); }

void shrink_and_clear() { static_cast<DerivedT *>(this)->shrink_and_clear(); }
Expand Down Expand Up @@ -1217,21 +1187,43 @@ class DenseMapIterator : DebugEpochBase::HandleBase {
pointer Ptr = nullptr;
pointer End = nullptr;

public:
DenseMapIterator() = default;

DenseMapIterator(pointer Pos, pointer E, const DebugEpochBase &Epoch,
bool NoAdvance = false)
DenseMapIterator(pointer Pos, pointer E, const DebugEpochBase &Epoch)
: DebugEpochBase::HandleBase(&Epoch), Ptr(Pos), End(E) {
assert(isHandleInSync() && "invalid construction!");
}

if (NoAdvance)
return;
public:
DenseMapIterator() = default;

static DenseMapIterator makeBegin(iterator_range<pointer> Buckets,
bool IsEmpty, const DebugEpochBase &Epoch) {
// When the map is empty, avoid the overhead of advancing/retreating past
// empty buckets.
if (IsEmpty)
return makeEnd(Buckets, Epoch);
if (shouldReverseIterate<KeyT>()) {
RetreatPastEmptyBuckets();
return;
DenseMapIterator Iter(Buckets.end(), Buckets.begin(), Epoch);
Iter.RetreatPastEmptyBuckets();
return Iter;
}
AdvancePastEmptyBuckets();
DenseMapIterator Iter(Buckets.begin(), Buckets.end(), Epoch);
Iter.AdvancePastEmptyBuckets();
return Iter;
}

static DenseMapIterator makeEnd(iterator_range<pointer> Buckets,
const DebugEpochBase &Epoch) {
if (shouldReverseIterate<KeyT>())
return DenseMapIterator(Buckets.begin(), Buckets.begin(), Epoch);
return DenseMapIterator(Buckets.end(), Buckets.end(), Epoch);
}

static DenseMapIterator makeIterator(pointer P,
iterator_range<pointer> Buckets,
const DebugEpochBase &Epoch) {
if (shouldReverseIterate<KeyT>())
return DenseMapIterator(P + 1, Buckets.begin(), Epoch);
return DenseMapIterator(P, Buckets.end(), Epoch);
}

// Converting ctor from non-const iterators to const iterators. SFINAE'd out
Expand Down