Skip to content
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

[C API] Add getters for Target Extension Types to C API #71291

Closed
wants to merge 5 commits into from

Conversation

Benjins
Copy link
Contributor

@Benjins Benjins commented Nov 4, 2023

These types were added in LLVM-16, and the C API supports constructing them, but not getting information back from them

This change adds getters for them to the C API, updates the echo test to be able to clone Target Extension Types, and adds some usage of them to the echo.ll test to confirm that they work

These types were added in LLVM-16, and the C API supports constructing them,
but not getting information back from them

This change adds getters for them to the C API, updates the echo test to be
able to clone Target Extension Types, and adds some usage of them to the
echo.ll test to confirm that they work
@llvmbot llvmbot added the llvm:ir label Nov 4, 2023
@llvmbot
Copy link
Member

llvmbot commented Nov 4, 2023

@llvm/pr-subscribers-llvm-ir

Author: Benji Smith (Benjins)

Changes

These types were added in LLVM-16, and the C API supports constructing them, but not getting information back from them

This change adds getters for them to the C API, updates the echo test to be able to clone Target Extension Types, and adds some usage of them to the echo.ll test to confirm that they work


Full diff: https://github.com/llvm/llvm-project/pull/71291.diff

4 Files Affected:

  • (modified) llvm/include/llvm-c/Core.h (+33)
  • (modified) llvm/lib/IR/Core.cpp (+32)
  • (modified) llvm/test/Bindings/llvm-c/echo.ll (+17)
  • (modified) llvm/tools/llvm-c-test/echo.cpp (+36-2)
diff --git a/llvm/include/llvm-c/Core.h b/llvm/include/llvm-c/Core.h
index 4fc88b2b64eacea..2d95f198f64958f 100644
--- a/llvm/include/llvm-c/Core.h
+++ b/llvm/include/llvm-c/Core.h
@@ -1654,6 +1654,39 @@ LLVMTypeRef LLVMTargetExtTypeInContext(LLVMContextRef C, const char *Name,
                                        unsigned *IntParams,
                                        unsigned IntParamCount);
 
+/**
+ * Obtain the name for this target extension type
+ */
+const char *LLVMGetTargetExtTypeName(LLVMTypeRef TargetExtTy);
+
+/**
+ * Obtain the number of type parameters for this target extension type
+ */
+unsigned LLVMCountTargetExtTypeTypeParams(LLVMTypeRef TargetExtTy);
+
+/**
+ * Obtain the values of a target extension type's type parameters, output to
+ * the passed-in pointer. The pointer should have enough space for all type
+ * params for the given target extension type
+ *
+ * @see LLVMCountTargetExtTypeTypeParams
+ */
+void LLVMGetTargetExtTypeTypeParams(LLVMTypeRef TargetExtTy, LLVMTypeRef *Dest);
+
+/**
+ * Obtain the number of int parameters for this target extension type
+ */
+unsigned LLVMCountTargetExtTypeIntParams(LLVMTypeRef TargetExtTy);
+
+/**
+ * Obtain the int values of a target extension type's int parameters, output to
+ * the passed-in pointer. The pointer should have enough space for all int
+ * params for the given target extension type
+ *
+ * @see LLVMCountTargetExtTypeIntParams
+ */
+void LLVMGetTargetExtTypeIntParams(LLVMTypeRef TargetExtTy, unsigned *Dest);
+
 /**
  * @}
  */
diff --git a/llvm/lib/IR/Core.cpp b/llvm/lib/IR/Core.cpp
index 076d1089582fe7e..eb1cda55288e334 100644
--- a/llvm/lib/IR/Core.cpp
+++ b/llvm/lib/IR/Core.cpp
@@ -924,6 +924,38 @@ LLVMTypeRef LLVMTargetExtTypeInContext(LLVMContextRef C, const char *Name,
       TargetExtType::get(*unwrap(C), Name, TypeParamArray, IntParamArray));
 }
 
+const char *LLVMGetTargetExtTypeName(LLVMTypeRef TargetExtTy) {
+  TargetExtType *Type = unwrap<TargetExtType>(TargetExtTy);
+  return Type->getName().data();
+}
+
+unsigned LLVMCountTargetExtTypeTypeParams(LLVMTypeRef TargetExtTy) {
+  TargetExtType *Type = unwrap<TargetExtType>(TargetExtTy);
+  return Type->getNumTypeParameters();
+}
+
+void LLVMGetTargetExtTypeTypeParams(LLVMTypeRef TargetExtTy,
+                                    LLVMTypeRef *Dest) {
+  TargetExtType *Type = unwrap<TargetExtType>(TargetExtTy);
+
+  for (unsigned int i = 0; i < Type->getNumTypeParameters(); i++) {
+    Dest[i] = wrap(Type->getTypeParameter(i));
+  }
+}
+
+unsigned LLVMCountTargetExtTypeIntParams(LLVMTypeRef TargetExtTy) {
+  TargetExtType *Type = unwrap<TargetExtType>(TargetExtTy);
+  return Type->getNumIntParameters();
+}
+
+void LLVMGetTargetExtTypeIntParams(LLVMTypeRef TargetExtTy, unsigned *Dest) {
+  TargetExtType *Type = unwrap<TargetExtType>(TargetExtTy);
+
+  for (unsigned int i = 0; i < Type->getNumIntParameters(); i++) {
+    Dest[i] = Type->getIntParameter(i);
+  }
+}
+
 /*===-- Operations on values ----------------------------------------------===*/
 
 /*--.. Operations on all values ............................................--*/
diff --git a/llvm/test/Bindings/llvm-c/echo.ll b/llvm/test/Bindings/llvm-c/echo.ll
index 5daa238bfb8e533..e62670424112d51 100644
--- a/llvm/test/Bindings/llvm-c/echo.ll
+++ b/llvm/test/Bindings/llvm-c/echo.ll
@@ -66,6 +66,23 @@ define void @types() {
   ret void
 }
 
+; Target extension types:
+define target("target.ext.1") @target_ext_01(target("target.ext.1") %0) {
+	ret target("target.ext.1") %0
+}
+
+define target("target.ext.2", i8, i1) @target_ext_02(target("target.ext.2", i8, i1) %0) {
+	ret target("target.ext.2", i8, i1) %0
+}
+
+define target("target.ext.3", 7) @target_ext_03(target("target.ext.3", 7) %0) {
+	ret target("target.ext.3", 7) %0
+}
+
+define target("target.ext.4", i1, i32, 7) @target_ext_04(target("target.ext.4", i1, i32, 7) %0) {
+	ret target("target.ext.4", i1, i32, 7) %0
+}
+
 define i32 @iops(i32 %a, i32 %b) {
   %1 = add i32 %a, %b
   %2 = mul i32 %a, %1
diff --git a/llvm/tools/llvm-c-test/echo.cpp b/llvm/tools/llvm-c-test/echo.cpp
index 06966ce528eae4d..8ff7bc1ed0bf485 100644
--- a/llvm/tools/llvm-c-test/echo.cpp
+++ b/llvm/tools/llvm-c-test/echo.cpp
@@ -157,8 +157,42 @@ struct TypeCloner {
         return LLVMX86MMXTypeInContext(Ctx);
       case LLVMTokenTypeKind:
         return LLVMTokenTypeInContext(Ctx);
-      case LLVMTargetExtTypeKind:
-        assert(false && "Implement me");
+      case LLVMTargetExtTypeKind: {
+        const char *Name = LLVMGetTargetExtTypeName(Src);
+        unsigned TypeParamCount = LLVMCountTargetExtTypeTypeParams(Src);
+        unsigned IntParamCount = LLVMCountTargetExtTypeIntParams(Src);
+
+        LLVMTypeRef *TypeParams = nullptr;
+        unsigned *IntParams = nullptr;
+
+        // If we have type params, get them from Src and clone them individually
+        if (TypeParamCount > 0) {
+          TypeParams = static_cast<LLVMTypeRef *>(
+              safe_malloc(TypeParamCount * sizeof(LLVMTypeRef)));
+          LLVMGetTargetExtTypeTypeParams(Src, TypeParams);
+
+          for (unsigned i = 0; i < TypeParamCount; i++)
+            TypeParams[i] = Clone(TypeParams[i]);
+        }
+
+        // If we have integer params, get them from Src
+        if (IntParamCount > 0) {
+          IntParams = static_cast<unsigned *>(
+              safe_malloc(IntParamCount * sizeof(unsigned)));
+          LLVMGetTargetExtTypeIntParams(Src, IntParams);
+        }
+
+        LLVMTypeRef TargtExtTy = LLVMTargetExtTypeInContext(
+            Ctx, Name, TypeParams, TypeParamCount, IntParams, IntParamCount);
+
+        if (TypeParams)
+          free(TypeParams);
+
+        if (IntParams)
+          free(IntParams);
+
+        return TargtExtTy;
+      }
     }
 
     fprintf(stderr, "%d is not a supported typekind\n", Kind);

Copy link
Contributor

@jcranmer-intel jcranmer-intel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs mentioning in the release notes.

@Benjins
Copy link
Contributor Author

Benjins commented Nov 6, 2023

This needs mentioning in the release notes.

Gotcha. I've added a section to the release notes for it in another commit: let me know if it's okay

@Benjins
Copy link
Contributor Author

Benjins commented Nov 13, 2023

Bumping this for review

Copy link
Contributor

@jcranmer-intel jcranmer-intel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the other methods, I think it would also help to have a get I'th type/integer parameter method too in the API.

Comment on lines 165 to 166
LLVMTypeRef *TypeParams = nullptr;
unsigned *IntParams = nullptr;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These types should be SmallVector<LLVMTypeRef, 4> and SmallVector<unsigned, 4>, respectively...

@Benjins
Copy link
Contributor Author

Benjins commented Nov 20, 2023

Looking at the other methods, I think it would also help to have a get I'th type/integer parameter method too in the API.
These types should be SmallVector<LLVMTypeRef, 4> and SmallVector<unsigned, 4>, respectively...

These should both be taken care of after the couple commits I pushed just now

@Benjins
Copy link
Contributor Author

Benjins commented Nov 30, 2023

Bumping this for review, after merging from main and resolving the conflicts

@Benjins
Copy link
Contributor Author

Benjins commented Jun 24, 2024

Superseded by #96447

@Benjins Benjins closed this Jun 24, 2024
nikic pushed a commit that referenced this pull request Jun 26, 2024
Accessors for the name, type parameters, and integer parameters are
added. A test is added to echo.ll

This was originally done in
#71291 but that has been stale
for several months. This re-applies the changes, but with some tweaks.
e.g. removing the bulk getters in favour of a simple get-by-index
approach for the type/integer parameters. The latter is more in line
with the rest of the API
lravenclaw pushed a commit to lravenclaw/llvm-project that referenced this pull request Jul 3, 2024
Accessors for the name, type parameters, and integer parameters are
added. A test is added to echo.ll

This was originally done in
llvm#71291 but that has been stale
for several months. This re-applies the changes, but with some tweaks.
e.g. removing the bulk getters in favour of a simple get-by-index
approach for the type/integer parameters. The latter is more in line
with the rest of the API
AlexisPerry pushed a commit to llvm-project-tlp/llvm-project that referenced this pull request Jul 9, 2024
Accessors for the name, type parameters, and integer parameters are
added. A test is added to echo.ll

This was originally done in
llvm#71291 but that has been stale
for several months. This re-applies the changes, but with some tweaks.
e.g. removing the bulk getters in favour of a simple get-by-index
approach for the type/integer parameters. The latter is more in line
with the rest of the API
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants