[IR] enable attaching metadata on ifuncs #158732
Conversation
|
@llvm/pr-subscribers-llvm-ir Author: Wael Yehia (w2yehia) ChangesIn PR #153049, we have a use case of attaching the Teach the IR parser and writer to support metadata on ifuncs, and update documentation. Full diff: https://github.com/llvm/llvm-project/pull/158732.diff 6 Files Affected:
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 6ba3759080cc3..d6b472af033f8 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -1020,13 +1020,14 @@ On ELF platforms, IFuncs are resolved by the dynamic linker at load time. On
Mach-O platforms, they are lowered in terms of ``.symbol_resolver`` functions,
which lazily resolve the callee the first time they are called.
-IFunc may have an optional :ref:`linkage type <linkage>` and an optional
-:ref:`visibility style <visibility>`.
+IFunc may have an optional :ref:`linkage type <linkage>`, an optional
+:ref:`visibility style <visibility>`, an option partition, and an optional
+list of attached :ref:`metadata <metadata>`.
Syntax::
@<Name> = [Linkage] [PreemptionSpecifier] [Visibility] ifunc <IFuncTy>, <ResolverTy>* @<Resolver>
- [, partition "name"]
+ [, partition "name"] (, !name !N)*
.. _langref_comdats:
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index 1bc2906f63b07..8739b24d4b74b 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -1258,6 +1258,9 @@ bool LLParser::parseAliasOrIFunc(const std::string &Name, unsigned NameID,
GV->setPartition(Lex.getStrVal());
if (parseToken(lltok::StringConstant, "expected partition string"))
return true;
+ } else if (!IsAlias && Lex.getKind() == lltok::MetadataVar) {
+ if (parseGlobalObjectMetadataAttachment(*GI.get()))
+ return true;
} else {
return tokError("unknown alias or ifunc property!");
}
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index a3f825408d0c2..d9e138edb8ce2 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -2630,6 +2630,9 @@ void ModuleBitcodeWriter::writeModuleMetadata() {
for (const Function &F : M)
if (F.isDeclaration() && F.hasMetadata())
AddDeclAttachedMetadata(F);
+ for (const GlobalIFunc &GI : M.ifuncs())
+ if (GI.hasMetadata())
+ AddDeclAttachedMetadata(GI);
// FIXME: Only store metadata for declarations here, and move data for global
// variable definitions to a separate block (PR28134).
for (const GlobalVariable &GV : M.globals())
diff --git a/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp b/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
index e133abe577c22..f497c574ee75d 100644
--- a/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
+++ b/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
@@ -495,6 +495,12 @@ ValueEnumerator::ValueEnumerator(const Module &M,
EnumerateMetadata(&F, Op);
}
}
+ for (const GlobalIFunc &GIF : M.ifuncs()) {
+ MDs.clear();
+ GIF.getAllMetadata(MDs);
+ for (const auto &I : MDs)
+ EnumerateMetadata(nullptr, I.second);
+ }
// Optimize constant ordering.
OptimizeConstants(FirstConstant, Values.size());
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index dc6d599fa9585..690dac4e6133b 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -1078,6 +1078,7 @@ void SlotTracker::processModule() {
for (const GlobalIFunc &I : TheModule->ifuncs()) {
if (!I.hasName())
CreateModuleSlot(&I);
+ processGlobalObjectMetadata(I);
}
// Add metadata used by named metadata.
@@ -4077,6 +4078,11 @@ void AssemblyWriter::printIFunc(const GlobalIFunc *GI) {
printEscapedString(GI->getPartition(), Out);
Out << '"';
}
+ SmallVector<std::pair<unsigned, MDNode *>, 4> MDs;
+ GI->getAllMetadata(MDs);
+ if (!MDs.empty()) {
+ printMetadataAttachments(MDs, ", ");
+ }
printInfoComment(*GI);
Out << '\n';
diff --git a/llvm/test/Assembler/metadata.ll b/llvm/test/Assembler/metadata.ll
index 5b62bfafa6d7d..b1fb720eb31f9 100644
--- a/llvm/test/Assembler/metadata.ll
+++ b/llvm/test/Assembler/metadata.ll
@@ -5,6 +5,14 @@
; CHECK-UNMAT: @global = global i32 0, !foo [[M2:![0-9]+]], !foo [[M3:![0-9]+]], !baz [[M3]]
@global = global i32 0, !foo !2, !foo !3, !baz !3
+; CHECK-UNMAT: @ifunc_func = ifunc void (...), ptr @resolver, !foo [[M2]]
+@ifunc_func = ifunc void (...), ptr @resolver, !foo !2
+
+define internal ptr @resolver() {
+entry:
+ ret ptr @test
+}
+
; CHECK-LABEL: @test
; CHECK: ret void, !foo [[M0:![0-9]+]], !bar [[M1:![0-9]+]]
define void @test() !dbg !1 {
|
In PR llvm#153049, we have a use case of attaching the !associated metadata to an ifunc. Since an ifunc is similar to a function declaration, it seems natural to allow metadata on ifuncs. Currently, the metadata API allows adding Metadata to llvm::Values, so the in-memory IR allows for metadata on ifuncs, but the IR reader/writer is not aware of that. Teach the IR parser and writer to support metadata on ifuncs, and update documentation.
LGMT but lets wait to see what others say. |
|
|
||
| void Verifier::visitGlobalIFunc(const GlobalIFunc &GI) { | ||
| visitGlobalValue(GI); | ||
|
|
There was a problem hiding this comment.
I suspect we need to do a bit more in the verifier. Particularly for !dbg. And maybe !associated. (Also probably should check that there's no section or comdat, but I guess that's an existing issue.)
I don't see any BitcodeReader changes; does that just work without any changes?
There was a problem hiding this comment.
I don't see any BitcodeReader changes; does that just work without any changes?
Yes, the BitcodeWriter saves the metadata on function declarations, global variables, and ifuncs (added in this PR) in a bitc::METADATA_GLOBAL_DECL_ATTACHMENT record, which IIUC creates an association between the metadata and the GlobalObject they belong to.
Function MetadataLoader::MetadataLoaderImpl::parseGlobalObjectAttachment in the BitcodeReader consumes the record and adds the metadata to the associated llvm::GlobalObject (which is a base class of Function, GlobalVariable, and GlobalIFunc).
There was a problem hiding this comment.
And maybe
!associated
What special property of an !associated on an ifunc should we check?
There was a problem hiding this comment.
Thinking a bit more, I guess there isn't anything specific to ifuncs for !associated.
|
@DanielKristofKiss @efriedma-quic Thanks alot for reviewing so promptly. |
|
I'm not very familiar with debug info. I tried the testcase from the PR that added |
|
I will wait for one more day for comment and merge if none appear. |
4 participants
Teach the IR parser and writer to support metadata on ifuncs, and update documentation.
In PR #153049, we have a use case of attaching the
!associatedmetadata to an ifunc.Since an ifunc is similar to a function declaration, it seems natural to allow metadata on ifuncs.
Currently, the metadata API allows adding Metadata to llvm::GlobalObject, so the in-memory IR allows for metadata on ifuncs, but the IR reader/writer is not aware of that.