[llvm] Metadata: Optimize metadata queries (PR #70700)
Matthias Braun via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 31 09:49:15 PDT 2023
https://github.com/MatzeB updated https://github.com/llvm/llvm-project/pull/70700
>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 1/3] 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(
>From 39a4dbfab33f776f6313b7d264e8ae62f70dd993 Mon Sep 17 00:00:00 2001
From: Matthias Braun <matze at braunis.de>
Date: Mon, 30 Oct 2023 13:02:15 -0700
Subject: [PATCH 2/3] avoid braces for 1-statement bodies
---
llvm/include/llvm/IR/Instruction.h | 6 ++----
llvm/lib/IR/Metadata.cpp | 6 ++----
2 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h
index fbeafd645a5a038..638bd7f7b1c79a3 100644
--- a/llvm/include/llvm/IR/Instruction.h
+++ b/llvm/include/llvm/IR/Instruction.h
@@ -304,12 +304,10 @@ class Instruction : public User,
/// If the metadata is not found then return null.
MDNode *getMetadata(unsigned KindID) const {
// Handle 'dbg' as a special case since it is not stored in the hash table.
- if (KindID == LLVMContext::MD_dbg) {
+ if (KindID == LLVMContext::MD_dbg)
return DbgLoc.getAsMDNode();
- }
- if (hasMetadataOtherThanDebugLoc()) {
+ if (hasMetadataOtherThanDebugLoc())
return getMetadataImpl(KindID);
- }
return nullptr;
}
diff --git a/llvm/lib/IR/Metadata.cpp b/llvm/lib/IR/Metadata.cpp
index a3f171fe88e983e..6e7cc0abf6d4234 100644
--- a/llvm/lib/IR/Metadata.cpp
+++ b/llvm/lib/IR/Metadata.cpp
@@ -1466,12 +1466,10 @@ void Instruction::setMetadata(StringRef Kind, MDNode *Node) {
MDNode *Instruction::getMetadataImpl(StringRef Kind) const {
const LLVMContext &Ctx = getContext();
unsigned KindID = Ctx.getMDKindID(Kind);
- if (KindID == LLVMContext::MD_dbg) {
+ if (KindID == LLVMContext::MD_dbg)
return DbgLoc.getAsMDNode();
- }
- if (hasMetadataOtherThanDebugLoc()) {
+ if (hasMetadataOtherThanDebugLoc())
return getValueMetadata(*this, KindID, Ctx);
- }
return nullptr;
}
>From 2a87a1655da0f4ea564b33d4233c4b69deb9e57e Mon Sep 17 00:00:00 2001
From: Matthias Braun <matze at braunis.de>
Date: Tue, 31 Oct 2023 09:30:41 -0700
Subject: [PATCH 3/3] Shuffle functions around: Eliminate
Instruction::getMetadataImpl in favor of new Value::getMetadataImpl
---
llvm/include/llvm/IR/Instruction.h | 5 +----
llvm/include/llvm/IR/Value.h | 11 ++++++++++-
llvm/lib/IR/Metadata.cpp | 27 +++++++--------------------
3 files changed, 18 insertions(+), 25 deletions(-)
diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h
index 638bd7f7b1c79a3..b5ccdf020a4c006 100644
--- a/llvm/include/llvm/IR/Instruction.h
+++ b/llvm/include/llvm/IR/Instruction.h
@@ -306,9 +306,7 @@ class Instruction : public User,
// 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;
+ return Value::getMetadata(KindID);
}
/// Get the metadata of given kind attached to this Instruction.
@@ -598,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 6e7cc0abf6d4234..7860280619bb941 100644
--- a/llvm/lib/IR/Metadata.cpp
+++ b/llvm/lib/IR/Metadata.cpp
@@ -56,12 +56,6 @@
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();
@@ -1357,18 +1351,17 @@ 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;
- return getValueMetadata(*this, KindID, getContext());
+ unsigned KindID = getContext().getMDKindID(Kind);
+ return getMetadataImpl(KindID);
}
-MDNode *Value::getMetadata(StringRef Kind) const {
- if (!hasMetadata())
- return nullptr;
+MDNode *Value::getMetadataImpl(unsigned KindID) const {
const LLVMContext &Ctx = getContext();
- unsigned KindID = Ctx.getMDKindID(Kind);
- return getValueMetadata(*this, KindID, Ctx);
+ const MDAttachments &Attachements = Ctx.pImpl->ValueMetadata.at(this);
+ return Attachements.lookup(KindID);
}
void Value::getMetadata(unsigned KindID, SmallVectorImpl<MDNode *> &MDs) const {
@@ -1468,9 +1461,7 @@ MDNode *Instruction::getMetadataImpl(StringRef Kind) const {
unsigned KindID = Ctx.getMDKindID(Kind);
if (KindID == LLVMContext::MD_dbg)
return DbgLoc.getAsMDNode();
- if (hasMetadataOtherThanDebugLoc())
- return getValueMetadata(*this, KindID, Ctx);
- return nullptr;
+ return Value::getMetadata(KindID);
}
void Instruction::dropUnknownNonDebugMetadata(ArrayRef<unsigned> KnownIDs) {
@@ -1628,10 +1619,6 @@ void Instruction::setNoSanitizeMetadata() {
llvm::MDNode::get(getContext(), std::nullopt));
}
-MDNode *Instruction::getMetadataImpl(unsigned KindID) const {
- return getValueMetadata(*this, KindID, getContext());
-}
-
void Instruction::getAllMetadataImpl(
SmallVectorImpl<std::pair<unsigned, MDNode *>> &Result) const {
Result.clear();
More information about the llvm-commits
mailing list