[llvm] af010e7 - Metadata: Optimize metadata queries (#70700)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 31 16:40:45 PDT 2023
Author: Matthias Braun
Date: 2023-10-31T16:40:41-07:00
New Revision: af010e79163b373e49fb45bf39878fd35d8dcd49
URL: https://github.com/llvm/llvm-project/commit/af010e79163b373e49fb45bf39878fd35d8dcd49
DIFF: https://github.com/llvm/llvm-project/commit/af010e79163b373e49fb45bf39878fd35d8dcd49.diff
LOG: Metadata: Optimize metadata queries (#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 and bad inlining
decisions for the unused insertion/growing code in `operator[]`.
- Avoid a redundant `Value::hasMetadata()` check.
- Move the `KindID == LLVMContext::MD_dbg` case to
`Instruction::getMetadata` and check it first assuming that it can be
constant folded after inlining 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.
Added:
Modified:
llvm/include/llvm/IR/Instruction.h
llvm/include/llvm/IR/Value.h
llvm/lib/IR/Metadata.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h
index 5142847fa75fff2..b5ccdf020a4c006 100644
--- a/llvm/include/llvm/IR/Instruction.h
+++ b/llvm/include/llvm/IR/Instruction.h
@@ -303,8 +303,10 @@ 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();
+ return Value::getMetadata(KindID);
}
/// Get the metadata of given kind attached to this Instruction.
@@ -594,7 +596,6 @@ class Instruction : public User,
private:
// These are all implemented in Metadata.cpp.
- MDNode *getMetadataImpl(unsigned KindID) const;
MDNode *getMetadataImpl(StringRef Kind) const;
void
getAllMetadataImpl(SmallVectorImpl<std::pair<unsigned, MDNode *>> &) const;
diff --git a/llvm/include/llvm/IR/Value.h b/llvm/include/llvm/IR/Value.h
index bc8284c068d0ed1..8fce56e12c52357 100644
--- a/llvm/include/llvm/IR/Value.h
+++ b/llvm/include/llvm/IR/Value.h
@@ -562,7 +562,11 @@ class Value {
/// These functions require that the value have at most a single attachment
/// of the given kind, and return \c nullptr if such an attachment is missing.
/// @{
- MDNode *getMetadata(unsigned KindID) const;
+ MDNode *getMetadata(unsigned KindID) const {
+ if (!HasMetadata)
+ return nullptr;
+ return getMetadataImpl(KindID);
+ }
MDNode *getMetadata(StringRef Kind) const;
/// @}
@@ -617,6 +621,11 @@ class Value {
/// Erase all metadata attached to this Value.
void clearMetadata();
+ /// Get metadata for the given kind, if any.
+ /// This is an internal function that must only be called after
+ /// checking that `hasMetadata()` returns true.
+ MDNode *getMetadataImpl(unsigned KindID) const;
+
public:
/// Return true if this value is a swifterror value.
///
diff --git a/llvm/lib/IR/Metadata.cpp b/llvm/lib/IR/Metadata.cpp
index c153ffb71a73bba..7860280619bb941 100644
--- a/llvm/lib/IR/Metadata.cpp
+++ b/llvm/lib/IR/Metadata.cpp
@@ -1351,25 +1351,22 @@ bool MDAttachments::erase(unsigned ID) {
return OldSize != Attachments.size();
}
-MDNode *Value::getMetadata(unsigned KindID) const {
+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(KindID);
+ unsigned KindID = getContext().getMDKindID(Kind);
+ return getMetadataImpl(KindID);
}
-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));
+MDNode *Value::getMetadataImpl(unsigned KindID) const {
+ const LLVMContext &Ctx = getContext();
+ const MDAttachments &Attachements = Ctx.pImpl->ValueMetadata.at(this);
+ return Attachements.lookup(KindID);
}
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 +1379,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 +1389,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 +1402,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 +1434,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 +1457,11 @@ 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();
+ return Value::getMetadata(KindID);
}
void Instruction::dropUnknownNonDebugMetadata(ArrayRef<unsigned> KnownIDs) {
@@ -1475,7 +1475,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 +1598,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);
@@ -1619,13 +1619,6 @@ void Instruction::setNoSanitizeMetadata() {
llvm::MDNode::get(getContext(), std::nullopt));
}
-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);
-}
-
void Instruction::getAllMetadataImpl(
SmallVectorImpl<std::pair<unsigned, MDNode *>> &Result) const {
Result.clear();
More information about the llvm-commits
mailing list