[clang] [llvm] [mlir] [ADT] Give DenseMapPair its own members instead of a std::pair base. NFC (PR #221853)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 7 17:03:47 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir
Author: Fangrui Song (MaskRay)
<details>
<summary>Changes</summary>
Some fast paths check `std::is_trivially_copyable_v<KeyT> &&
std::is_trivially_copyable_v<ValueT>` instead of the bucket type,
because std::pair has a non-trivial copy assignment (which costs trivial
copyability). Hold the members directly, making the bucket trivially
copyable.
instructions:u in a stage-2 clang build decreases by 0.35%, likely due
to saving std::pair instantiations (std::pair implementations have
expensive `enable_if`)
---
Full diff: https://github.com/llvm/llvm-project/pull/221853.diff
10 Files Affected:
- (modified) clang/lib/Sema/SemaAttr.cpp (+1-1)
- (modified) llvm/include/llvm/ADT/DenseMap.h (+37-17)
- (modified) llvm/lib/CodeGen/RegisterUsageInfo.cpp (+1-1)
- (modified) llvm/lib/MC/StringTableBuilder.cpp (+1-1)
- (modified) llvm/lib/MCA/HardwareUnits/LSUnit.cpp (+1-1)
- (modified) llvm/lib/MCA/HardwareUnits/ResourceManager.cpp (+1-1)
- (modified) llvm/lib/Transforms/Scalar/GVNHoist.cpp (+1-1)
- (modified) llvm/lib/Transforms/Scalar/GVNSink.cpp (+4-1)
- (modified) llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp (+1-3)
- (modified) mlir/lib/Conversion/SCFToGPU/SCFToGPU.cpp (+2-3)
``````````diff
diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp
index 67573c9f1c72a..35d14a4444595 100644
--- a/clang/lib/Sema/SemaAttr.cpp
+++ b/clang/lib/Sema/SemaAttr.cpp
@@ -1074,7 +1074,7 @@ void Sema::ActOnPragmaAttributeAttribute(
// variable(is_parameter).
// - a sub-rule and a sibling that's negated. E.g.
// variable(is_thread_local) and variable(unless(is_parameter))
- llvm::SmallDenseMap<int, std::pair<int, SourceRange>, 2>
+ llvm::SmallDenseMap<int, attr::ParsedSubjectMatchRuleSet::value_type, 2>
RulesToFirstSpecifiedNegatedSubRule;
for (const auto &Rule : Rules) {
attr::SubjectMatchRule MatchRule = attr::SubjectMatchRule(Rule.first);
diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h
index 3073ab0ec9712..5ea7ce2aeff85 100644
--- a/llvm/include/llvm/ADT/DenseMap.h
+++ b/llvm/include/llvm/ADT/DenseMap.h
@@ -44,17 +44,39 @@
namespace llvm {
namespace detail {
+// A bucket holds a key and a value. Don't use std::pair, which has a
+// non-trivial copy assignment, which costs is_trivially_copyable.
+template <typename KeyT, typename ValueT> struct DenseMapPair {
+ using first_type = KeyT;
+ using second_type = ValueT;
+
+ KeyT first;
+ ValueT second;
+
+ DenseMapPair() : first(), second() {}
+ DenseMapPair(const KeyT &Key, const ValueT &Value)
+ : first(Key), second(Value) {}
+ DenseMapPair(KeyT &&Key, ValueT &&Value)
+ : first(std::move(Key)), second(std::move(Value)) {}
+ DenseMapPair(const std::pair<KeyT, ValueT> &P)
+ : first(P.first), second(P.second) {}
+ DenseMapPair(std::pair<KeyT, ValueT> &&P)
+ : first(std::move(P.first)), second(std::move(P.second)) {}
+
+ operator std::pair<KeyT, ValueT>() const { return {first, second}; }
+ operator std::pair<const KeyT, ValueT>() const { return {first, second}; }
+
+ friend bool operator==(const DenseMapPair &LHS, const DenseMapPair &RHS) {
+ return LHS.first == RHS.first && LHS.second == RHS.second;
+ }
+ friend bool operator!=(const DenseMapPair &LHS, const DenseMapPair &RHS) {
+ return !(LHS == RHS);
+ }
-// We extend a pair to allow users to override the bucket type with their own
-// implementation without requiring two members.
-template <typename KeyT, typename ValueT>
-struct DenseMapPair : std::pair<KeyT, ValueT> {
- using std::pair<KeyT, ValueT>::pair;
-
- KeyT &getFirst() { return std::pair<KeyT, ValueT>::first; }
- const KeyT &getFirst() const { return std::pair<KeyT, ValueT>::first; }
- ValueT &getSecond() { return std::pair<KeyT, ValueT>::second; }
- const ValueT &getSecond() const { return std::pair<KeyT, ValueT>::second; }
+ KeyT &getFirst() { return first; }
+ const KeyT &getFirst() const { return first; }
+ ValueT &getSecond() { return second; }
+ const ValueT &getSecond() const { return second; }
};
} // end namespace detail
@@ -330,7 +352,7 @@ class DenseMapBase : public DebugEpochBase {
/// Range insertion of pairs.
template <typename InputIt> void insert(InputIt I, InputIt E) {
for (; I != E; ++I)
- insert(*I);
+ try_emplace(I->first, I->second);
}
/// Inserts range of 'std::pair<KeyT, ValueT>' values into the map.
@@ -462,10 +484,9 @@ class DenseMapBase : public DebugEpochBase {
}
void destroyAll() {
- // No need to iterate through the buckets if both KeyT and ValueT are
- // trivially destructible.
- if constexpr (std::is_trivially_destructible_v<KeyT> &&
- std::is_trivially_destructible_v<ValueT>)
+ // No need to iterate through the buckets if the bucket is trivially
+ // destructible.
+ if constexpr (std::is_trivially_destructible_v<BucketT>)
return;
if (getNumBuckets() == 0) // Nothing to do.
@@ -555,8 +576,7 @@ class DenseMapBase : public DebugEpochBase {
const UsedT *OtherU = other.getUsed();
std::memcpy(U, OtherU,
llvm::densemap::detail::usedWords(NumBuckets) * sizeof(UsedT));
- if constexpr (std::is_trivially_copyable_v<KeyT> &&
- std::is_trivially_copyable_v<ValueT>) {
+ if constexpr (std::is_trivially_copyable_v<BucketT>) {
memcpy(reinterpret_cast<void *>(Buckets), OtherBuckets,
NumBuckets * sizeof(BucketT));
} else {
diff --git a/llvm/lib/CodeGen/RegisterUsageInfo.cpp b/llvm/lib/CodeGen/RegisterUsageInfo.cpp
index 38e4c30ceb634..d139bd430fb20 100644
--- a/llvm/lib/CodeGen/RegisterUsageInfo.cpp
+++ b/llvm/lib/CodeGen/RegisterUsageInfo.cpp
@@ -70,7 +70,7 @@ PhysicalRegisterUsageInfo::getRegUsageInfo(const Function &FP) {
}
void PhysicalRegisterUsageInfo::print(raw_ostream &OS, const Module *M) const {
- using FuncPtrRegMaskPair = std::pair<const Function *, std::vector<uint32_t>>;
+ using FuncPtrRegMaskPair = decltype(RegMasks)::value_type;
// Create a vector of pointer to RegMasks entries
SmallVector<const FuncPtrRegMaskPair *, 64> FPRMPairVector(
diff --git a/llvm/lib/MC/StringTableBuilder.cpp b/llvm/lib/MC/StringTableBuilder.cpp
index eb1a62f1f6412..39d10ad128ed7 100644
--- a/llvm/lib/MC/StringTableBuilder.cpp
+++ b/llvm/lib/MC/StringTableBuilder.cpp
@@ -66,7 +66,7 @@ void StringTableBuilder::write(raw_ostream &OS) const {
OS << Data;
}
-using StringPair = std::pair<CachedHashStringRef, size_t>;
+using StringPair = DenseMap<CachedHashStringRef, size_t>::value_type;
void StringTableBuilder::write(uint8_t *Buf) const {
assert(isFinalized());
diff --git a/llvm/lib/MCA/HardwareUnits/LSUnit.cpp b/llvm/lib/MCA/HardwareUnits/LSUnit.cpp
index bf0b432524881..f88f82a7f5279 100644
--- a/llvm/lib/MCA/HardwareUnits/LSUnit.cpp
+++ b/llvm/lib/MCA/HardwareUnits/LSUnit.cpp
@@ -42,7 +42,7 @@ LSUnitBase::LSUnitBase(const MCSchedModel &SM, unsigned LQ, unsigned SQ,
LSUnitBase::~LSUnitBase() = default;
void LSUnit::cycleEvent() {
- for (const std::pair<unsigned, std::unique_ptr<MemoryGroup>> &G : Groups)
+ for (const auto &G : Groups)
G.second->cycleEvent();
}
diff --git a/llvm/lib/MCA/HardwareUnits/ResourceManager.cpp b/llvm/lib/MCA/HardwareUnits/ResourceManager.cpp
index cdf3439e07d61..12d062ab6ff9f 100644
--- a/llvm/lib/MCA/HardwareUnits/ResourceManager.cpp
+++ b/llvm/lib/MCA/HardwareUnits/ResourceManager.cpp
@@ -473,7 +473,7 @@ void ResourceManager::fastIssueInstruction(
}
void ResourceManager::cycleEvent(SmallVectorImpl<ResourceRef> &ResourcesFreed) {
- for (std::pair<ResourceRef, unsigned> &BR : BusyResources) {
+ for (auto &BR : BusyResources) {
if (BR.second)
BR.second--;
if (!BR.second) {
diff --git a/llvm/lib/Transforms/Scalar/GVNHoist.cpp b/llvm/lib/Transforms/Scalar/GVNHoist.cpp
index 37562a024a2b0..6bb6d6772fbfb 100644
--- a/llvm/lib/Transforms/Scalar/GVNHoist.cpp
+++ b/llvm/lib/Transforms/Scalar/GVNHoist.cpp
@@ -835,7 +835,7 @@ void GVNHoist::findHoistableCandidates(OutValuesType &CHIBBs,
// CHIArgs now have the outgoing values, so check for anticipability and
// accumulate hoistable candidates in HPL.
- for (std::pair<BasicBlock *, SmallVector<CHIArg, 2>> &A : CHIBBs) {
+ for (auto &A : CHIBBs) {
BasicBlock *BB = A.first;
SmallVectorImpl<CHIArg> &CHIs = A.second;
// Vector of PHIs contains PHIs for different instructions.
diff --git a/llvm/lib/Transforms/Scalar/GVNSink.cpp b/llvm/lib/Transforms/Scalar/GVNSink.cpp
index 67196ef9715f1..eb7a9c68b5b61 100644
--- a/llvm/lib/Transforms/Scalar/GVNSink.cpp
+++ b/llvm/lib/Transforms/Scalar/GVNSink.cpp
@@ -606,7 +606,10 @@ GVNSink::analyzeInstructionForSinking(LockstepReverseIterator<false> &LRI,
return std::nullopt;
VNums[N]++;
}
- unsigned VNumToSink = llvm::max_element(VNums, llvm::less_second())->first;
+ unsigned VNumToSink =
+ llvm::max_element(VNums, [](const auto &L, const auto &R) {
+ return L.second < R.second;
+ })->first;
if (VNums[VNumToSink] == 1)
// Can't sink anything!
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 5de71f3bf87b5..d6edb1a2cee80 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -4480,9 +4480,7 @@ class slpvectorizer::BoUpSLP {
} while (It != P.first->Scalars.end());
}
return all_of(PotentiallyReorderedEntriesCount,
- [&](const std::pair<const TreeEntry *, unsigned> &P) {
- return P.second == NumOps - 1;
- });
+ [&](const auto &P) { return P.second == NumOps - 1; });
}
SmallVector<ScheduleCopyableData *>
diff --git a/mlir/lib/Conversion/SCFToGPU/SCFToGPU.cpp b/mlir/lib/Conversion/SCFToGPU/SCFToGPU.cpp
index 370457c85e797..1fbadcc4eff72 100644
--- a/mlir/lib/Conversion/SCFToGPU/SCFToGPU.cpp
+++ b/mlir/lib/Conversion/SCFToGPU/SCFToGPU.cpp
@@ -760,9 +760,8 @@ ParallelToGpuLaunchLowering::matchAndRewrite(ParallelOp parallelOp,
// Now that we succeeded creating the launch operation, also update the
// bounds.
- for (auto bound : launchBounds)
- launchOp.setOperand(getLaunchOpArgumentNum(std::get<0>(bound)),
- std::get<1>(bound));
+ for (const auto &bound : launchBounds)
+ launchOp.setOperand(getLaunchOpArgumentNum(bound.first), bound.second);
rewriter.eraseOp(parallelOp);
return success();
``````````
</details>
https://github.com/llvm/llvm-project/pull/221853
More information about the cfe-commits
mailing list