[llvm] Metadata: Optimize metadata queries (PR #70700)
Matthias Braun via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 30 11:33:16 PDT 2023
https://github.com/MatzeB created https://github.com/llvm/llvm-project/pull/70700
Optimize metadata query code:
- Avoid `DenseMap::operator[]` in situations where it is known that the key exists in the map. Instead use `DenseMap::at()`/ `DenseMap::find()->second`. This avoids code-bloat leading to missed inlining opportunities just because the unnecessary map growing code was around.
- Directly query the map in `Instruction::getMetadataImpl` instead of using `Value::getMetadata` to avoid an unnecessary `hasMetadata()` check.
- Move `KindID == LLVMContext::MD_dbg` to `Instruction::getMetadata` and check this case first assuming that it can be inlined and constant folded in many situations.
The motivation for this change is a regression triggered by e3cf80c5c1fe55efd8216575ccadea0ab087e79c which could attributed to the compiler inlining the insertion part of `DenseMap::operator[]` in more cases while unbeknownst to a compiler (without PGO) that code is never used in this context. This change improves performance and eliminates difference before and after that change in my measurements.
>From 181ea7fd3b334d5b866d4923d6fee784100a371f Mon Sep 17 00:00:00 2001
From: Matthias Braun <matze at braunis.de>
Date: Fri, 27 Oct 2023 16:35:47 -0700
Subject: [PATCH] Metadata: Optimize metadata queries
Optimize metadata query code:
- Avoid `DenseMap::operator[]` in situations where it is known that the
key exists in the map. Instead use `DenseMap::at()`/
`DenseMap::find()->second`. This avoids code-bloat leading to missed
inlining opportunities just because the unnecessary map growing code
was around.
- Directly query the map in `Instruction::getMetadataImpl` instead of
using `Value::getMetadata` to avoid an unnecessary `hasMetadata()`
check.
- Move `KindID == LLVMContext::MD_dbg` to `Instruction::getMetadata`
and check this case first assuming that it can be inlined and constant
folded in many situations.
---
llvm/include/llvm/IR/Instruction.h | 10 +++++--
llvm/lib/IR/Metadata.cpp | 46 ++++++++++++++++++------------
2 files changed, 35 insertions(+), 21 deletions(-)
diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h
index 5142847fa75fff2..fbeafd645a5a038 100644
--- a/llvm/include/llvm/IR/Instruction.h
+++ b/llvm/include/llvm/IR/Instruction.h
@@ -303,8 +303,14 @@ class Instruction : public User,
/// Get the metadata of given kind attached to this Instruction.
/// If the metadata is not found then return null.
MDNode *getMetadata(unsigned KindID) const {
- if (!hasMetadata()) return nullptr;
- return getMetadataImpl(KindID);
+ // Handle 'dbg' as a special case since it is not stored in the hash table.
+ if (KindID == LLVMContext::MD_dbg) {
+ return DbgLoc.getAsMDNode();
+ }
+ if (hasMetadataOtherThanDebugLoc()) {
+ return getMetadataImpl(KindID);
+ }
+ return nullptr;
}
/// Get the metadata of given kind attached to this Instruction.
diff --git a/llvm/lib/IR/Metadata.cpp b/llvm/lib/IR/Metadata.cpp
index c153ffb71a73bba..a3f171fe88e983e 100644
--- a/llvm/lib/IR/Metadata.cpp
+++ b/llvm/lib/IR/Metadata.cpp
@@ -56,6 +56,12 @@
using namespace llvm;
+static inline MDNode *getValueMetadata(const Value &Value, unsigned KindID,
+ const LLVMContext &Ctx) {
+ const MDAttachments &Attachements = Ctx.pImpl->ValueMetadata.at(&Value);
+ return Attachements.lookup(KindID);
+}
+
MetadataAsValue::MetadataAsValue(Type *Ty, Metadata *MD)
: Value(Ty, MetadataAsValueVal), MD(MD) {
track();
@@ -1354,22 +1360,20 @@ bool MDAttachments::erase(unsigned ID) {
MDNode *Value::getMetadata(unsigned KindID) const {
if (!hasMetadata())
return nullptr;
- const auto &Info = getContext().pImpl->ValueMetadata[this];
- assert(!Info.empty() && "bit out of sync with hash table");
- return Info.lookup(KindID);
+ return getValueMetadata(*this, KindID, getContext());
}
MDNode *Value::getMetadata(StringRef Kind) const {
if (!hasMetadata())
return nullptr;
- const auto &Info = getContext().pImpl->ValueMetadata[this];
- assert(!Info.empty() && "bit out of sync with hash table");
- return Info.lookup(getContext().getMDKindID(Kind));
+ const LLVMContext &Ctx = getContext();
+ unsigned KindID = Ctx.getMDKindID(Kind);
+ return getValueMetadata(*this, KindID, Ctx);
}
void Value::getMetadata(unsigned KindID, SmallVectorImpl<MDNode *> &MDs) const {
if (hasMetadata())
- getContext().pImpl->ValueMetadata[this].get(KindID, MDs);
+ getContext().pImpl->ValueMetadata.at(this).get(KindID, MDs);
}
void Value::getMetadata(StringRef Kind, SmallVectorImpl<MDNode *> &MDs) const {
@@ -1382,8 +1386,7 @@ void Value::getAllMetadata(
if (hasMetadata()) {
assert(getContext().pImpl->ValueMetadata.count(this) &&
"bit out of sync with hash table");
- const auto &Info = getContext().pImpl->ValueMetadata.find(this)->second;
- assert(!Info.empty() && "Shouldn't have called this");
+ const MDAttachments &Info = getContext().pImpl->ValueMetadata.at(this);
Info.getAll(MDs);
}
}
@@ -1393,7 +1396,7 @@ void Value::setMetadata(unsigned KindID, MDNode *Node) {
// Handle the case when we're adding/updating metadata on a value.
if (Node) {
- auto &Info = getContext().pImpl->ValueMetadata[this];
+ MDAttachments &Info = getContext().pImpl->ValueMetadata[this];
assert(!Info.empty() == HasMetadata && "bit out of sync with hash table");
if (Info.empty())
HasMetadata = true;
@@ -1406,7 +1409,7 @@ void Value::setMetadata(unsigned KindID, MDNode *Node) {
"bit out of sync with hash table");
if (!HasMetadata)
return; // Nothing to remove!
- auto &Info = getContext().pImpl->ValueMetadata[this];
+ MDAttachments &Info = getContext().pImpl->ValueMetadata.find(this)->second;
// Handle removal of an existing value.
Info.erase(KindID);
@@ -1438,7 +1441,7 @@ bool Value::eraseMetadata(unsigned KindID) {
if (!HasMetadata)
return false;
- auto &Store = getContext().pImpl->ValueMetadata[this];
+ MDAttachments &Store = getContext().pImpl->ValueMetadata.find(this)->second;
bool Changed = Store.erase(KindID);
if (Store.empty())
clearMetadata();
@@ -1461,7 +1464,15 @@ void Instruction::setMetadata(StringRef Kind, MDNode *Node) {
}
MDNode *Instruction::getMetadataImpl(StringRef Kind) const {
- return getMetadataImpl(getContext().getMDKindID(Kind));
+ const LLVMContext &Ctx = getContext();
+ unsigned KindID = Ctx.getMDKindID(Kind);
+ if (KindID == LLVMContext::MD_dbg) {
+ return DbgLoc.getAsMDNode();
+ }
+ if (hasMetadataOtherThanDebugLoc()) {
+ return getValueMetadata(*this, KindID, Ctx);
+ }
+ return nullptr;
}
void Instruction::dropUnknownNonDebugMetadata(ArrayRef<unsigned> KnownIDs) {
@@ -1475,7 +1486,7 @@ void Instruction::dropUnknownNonDebugMetadata(ArrayRef<unsigned> KnownIDs) {
KnownSet.insert(LLVMContext::MD_DIAssignID);
auto &MetadataStore = getContext().pImpl->ValueMetadata;
- auto &Info = MetadataStore[this];
+ MDAttachments &Info = MetadataStore.find(this)->second;
assert(!Info.empty() && "bit out of sync with hash table");
Info.remove_if([&KnownSet](const MDAttachments::Attachment &I) {
return !KnownSet.count(I.MDKind);
@@ -1598,7 +1609,7 @@ AAMDNodes Instruction::getAAMetadata() const {
// Not using Instruction::hasMetadata() because we're not interested in
// DebugInfoMetadata.
if (Value::hasMetadata()) {
- const auto &Info = getContext().pImpl->ValueMetadata[this];
+ const MDAttachments &Info = getContext().pImpl->ValueMetadata.at(this);
Result.TBAA = Info.lookup(LLVMContext::MD_tbaa);
Result.TBAAStruct = Info.lookup(LLVMContext::MD_tbaa_struct);
Result.Scope = Info.lookup(LLVMContext::MD_alias_scope);
@@ -1620,10 +1631,7 @@ void Instruction::setNoSanitizeMetadata() {
}
MDNode *Instruction::getMetadataImpl(unsigned KindID) const {
- // Handle 'dbg' as a special case since it is not stored in the hash table.
- if (KindID == LLVMContext::MD_dbg)
- return DbgLoc.getAsMDNode();
- return Value::getMetadata(KindID);
+ return getValueMetadata(*this, KindID, getContext());
}
void Instruction::getAllMetadataImpl(
More information about the llvm-commits
mailing list