llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-llvm-mc @llvm/pr-subscribers-llvm-regalloc 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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
