-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[LLVM][C API] Support Value extraction from a MetadataAsValue. #105501
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-llvm-ir Author: Tim Besard (maleadt) ChangesIt is already possible to extract the Metadata node contained in a MetadataAsValue using the C API: julia> md = MDString("foo")
!"foo"
julia> mav = Value(LLVM.API.LLVMMetadataAsValue(context(), md));
julia> typeof(Metadata(LLVM.API.LLVMValueAsMetadata(mav)))
MDString
julia> Metadata(LLVM.API.LLVMValueAsMetadata(mav)) == md
true However, julia> val = PointerNull(LLVM.PointerType(LLVM.Int8Type()))
i8* null
julia> vam = Metadata(LLVM.API.LLVMValueAsMetadata(val));
julia> typeof(Value(LLVM.API.LLVMMetadataAsValue(context(), vam)))
LLVM.MetadataAsValue
julia> Value(LLVM.API.LLVMMetadataAsValue(context(), vam)) == val
false This PR makes Full diff: https://github.com/llvm/llvm-project/pull/105501.diff 4 Files Affected:
diff --git a/llvm/lib/IR/Core.cpp b/llvm/lib/IR/Core.cpp
index 7665385025bd91..f4708bc18b3f8b 100644
--- a/llvm/lib/IR/Core.cpp
+++ b/llvm/lib/IR/Core.cpp
@@ -1328,8 +1328,12 @@ LLVMValueRef LLVMMDNode(LLVMValueRef *Vals, unsigned Count) {
return LLVMMDNodeInContext(LLVMGetGlobalContext(), Vals, Count);
}
-LLVMValueRef LLVMMetadataAsValue(LLVMContextRef C, LLVMMetadataRef MD) {
- return wrap(MetadataAsValue::get(*unwrap(C), unwrap(MD)));
+LLVMValueRef LLVMMetadataAsValue(LLVMContextRef C, LLVMMetadataRef Metadata) {
+ auto *MD = unwrap(Metadata);
+ if (auto *VAM = dyn_cast<ValueAsMetadata>(MD))
+ return wrap(VAM->getValue());
+ else
+ return wrap(MetadataAsValue::get(*unwrap(C), MD));
}
LLVMMetadataRef LLVMValueAsMetadata(LLVMValueRef Val) {
diff --git a/llvm/test/Bindings/llvm-c/vam_mav_extract.ll b/llvm/test/Bindings/llvm-c/vam_mav_extract.ll
new file mode 100644
index 00000000000000..31640c460ceab7
--- /dev/null
+++ b/llvm/test/Bindings/llvm-c/vam_mav_extract.ll
@@ -0,0 +1,2 @@
+; RUN: llvm-c-test --vam-mav-extract < /dev/null
+; This used to trigger an assertion
diff --git a/llvm/tools/llvm-c-test/llvm-c-test.h b/llvm/tools/llvm-c-test/llvm-c-test.h
index 1da6596cd5a8f2..44790e6a3bc8b5 100644
--- a/llvm/tools/llvm-c-test/llvm-c-test.h
+++ b/llvm/tools/llvm-c-test/llvm-c-test.h
@@ -45,6 +45,7 @@ int llvm_add_named_metadata_operand(void);
int llvm_set_metadata(void);
int llvm_replace_md_operand(void);
int llvm_is_a_value_as_metadata(void);
+int llvm_vam_mav_extract(void);
// object.c
int llvm_object_list_sections(void);
diff --git a/llvm/tools/llvm-c-test/main.c b/llvm/tools/llvm-c-test/main.c
index 8be9ea06fc68da..5a1f792fb941a3 100644
--- a/llvm/tools/llvm-c-test/main.c
+++ b/llvm/tools/llvm-c-test/main.c
@@ -56,6 +56,8 @@ static void print_usage(void) {
fprintf(stderr, " * --is-a-value-as-metadata\n");
fprintf(stderr,
" Run test for checking if LLVMValueRef is a ValueAsMetadata\n");
+ fprintf(stderr, " * --vam-mav-extract\n");
+ fprintf(stderr, " Run test for extracting data from ValueAsMetadata and MetadataAsValue\n");
fprintf(stderr, " * --echo\n");
fprintf(stderr, " Read bitcode file from stdin - print it back out\n\n");
fprintf(stderr, " * --test-diagnostic-handler\n");
@@ -101,6 +103,8 @@ int main(int argc, char **argv) {
return llvm_replace_md_operand();
} else if (argc == 2 && !strcmp(argv[1], "--is-a-value-as-metadata")) {
return llvm_is_a_value_as_metadata();
+ } else if (argc == 2 && !strcmp(argv[1], "--vam-mav-extract")) {
+ return llvm_vam_mav_extract();
} else if (argc == 2 && !strcmp(argv[1], "--test-function-attributes")) {
return llvm_test_function_attributes();
} else if (argc == 2 && !strcmp(argv[1], "--test-callsite-attributes")) {
diff --git a/llvm/tools/llvm-c-test/metadata.c b/llvm/tools/llvm-c-test/metadata.c
index 4fe8c00c57481b..a08ebacb49696e 100644
--- a/llvm/tools/llvm-c-test/metadata.c
+++ b/llvm/tools/llvm-c-test/metadata.c
@@ -80,7 +80,7 @@ int llvm_is_a_value_as_metadata(void) {
LLVMContextRef Context = LLVMGetModuleContext(M);
{
- LLVMValueRef Int = LLVMConstInt(LLVMInt32Type(), 0, 0);
+ LLVMValueRef Int = LLVMConstInt(LLVMInt32TypeInContext(Context), 0, 0);
LLVMValueRef NodeMD = LLVMMDNode(&Int, 1);
assert(LLVMIsAValueAsMetadata(NodeMD) == NodeMD);
(void)NodeMD;
@@ -98,3 +98,21 @@ int llvm_is_a_value_as_metadata(void) {
return 0;
}
+
+int llvm_vam_mav_extract(void) {
+ LLVMModuleRef M = LLVMModuleCreateWithName("Mod");
+ LLVMContextRef Context = LLVMGetModuleContext(M);
+
+ LLVMValueRef Val = LLVMConstInt(LLVMInt32TypeInContext(Context), 0, 0);
+ LLVMMetadataRef MD = LLVMMDStringInContext2(Context, "foo", 3);
+
+ // construction
+ LLVMValueRef MAV = LLVMMetadataAsValue(Context, MD);
+ LLVMMetadataRef VAM = LLVMValueAsMetadata(Val);
+
+ // extraction
+ assert(LLVMMetadataAsValue(Context, VAM) == Val);
+ assert(LLVMValueAsMetadata(MAV) == MD);
+
+ return 0;
+}
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
It is already possible to extract the underlying Metadata from a ValueAsMetadata.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new behavior looks reasonable to me, but I'm a bit concerned that this is a silent behavior change in the C API.
I guess that's fair. It seemed silly to create a Or should we switch to a more explicit API for extracting contained values from a ValueAsMetadata / metadata from a MetadataAsValue? I always found the explicit unwrapping behavior of |
It is already possible to extract the Metadata node contained in a MetadataAsValue using the C API:
However,
LLVMMetadataAsValue
currently does not support getting the value from aValueAsMetadata
:This PR makes
LLVMValueAsMetadata
behave likeLLVMMetadataAsValue
, returning the contained data when possible.We've been carrying this patch for a while in LLVM.jl.