Skip to content

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

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

Merged
merged 2 commits into from
Jun 26, 2024

Conversation

Benjins
Copy link
Contributor

@Benjins Benjins commented Jun 24, 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

@llvmbot
Copy link
Member

llvmbot commented Jun 24, 2024

@llvm/pr-subscribers-llvm-ir

Author: Benji Smith (Benjins)

Changes

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


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

5 Files Affected:

  • (modified) llvm/docs/ReleaseNotes.rst (+6)
  • (modified) llvm/include/llvm-c/Core.h (+36)
  • (modified) llvm/lib/IR/Core.cpp (+26)
  • (modified) llvm/test/Bindings/llvm-c/echo.ll (+17)
  • (modified) llvm/tools/llvm-c-test/echo.cpp (+20-2)
diff --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst
index 76356dd76f1d2..416b3952f1ac4 100644
--- a/llvm/docs/ReleaseNotes.rst
+++ b/llvm/docs/ReleaseNotes.rst
@@ -282,6 +282,12 @@ They are described in detail in the `debug info migration guide <https://llvm.or
   * ``LLVMDIBuilderInsertDbgValueBefore``
   * ``LLVMDIBuilderInsertDbgValueAtEnd``
 
+* Added the following functions for accessing a Target Extension Type's data:
+
+  * ``LLVMGetTargetExtTypeName``
+  * ``LLVMGetTargetExtTypeNumTypeParams``/``LLVMGetTargetExtTypeTypeParam``
+  * ``LLVMGetTargetExtTypeNumIntParams``/``LLVMGetTargetExtTypeIntParam``
+
 Changes to the CodeGen infrastructure
 -------------------------------------
 
diff --git a/llvm/include/llvm-c/Core.h b/llvm/include/llvm-c/Core.h
index af2e562fe0f8d..9867db4839fe1 100644
--- a/llvm/include/llvm-c/Core.h
+++ b/llvm/include/llvm-c/Core.h
@@ -1710,6 +1710,42 @@ LLVMTypeRef LLVMTargetExtTypeInContext(LLVMContextRef C, const char *Name,
                                        unsigned *IntParams,
                                        unsigned IntParamCount);
 
+/**
+ * Obtain the name for this target extension type.
+ *
+ * @see llvm::TargetExtType::getName()
+ */
+const char *LLVMGetTargetExtTypeName(LLVMTypeRef TargetExtTy);
+
+/**
+ * Obtain the number of type parameters for this target extension type.
+ *
+ * @see llvm::TargetExtType::getNumTypeParameters()
+ */
+unsigned LLVMGetTargetExtTypeNumTypeParams(LLVMTypeRef TargetExtTy);
+
+/**
+ * Get the type parameter at the given index for the target extension type.
+ *
+ * @see llvm::TargetExtType::getTypeParameter()
+ */
+LLVMTypeRef LLVMGetTargetExtTypeTypeParam(LLVMTypeRef TargetExtTy,
+                                          unsigned Idx);
+
+/**
+ * Obtain the number of int parameters for this target extension type.
+ *
+ * @see llvm::TargetExtType::getNumIntParameters()
+ */
+unsigned LLVMGetTargetExtTypeNumIntParams(LLVMTypeRef TargetExtTy);
+
+/**
+ * Get the int parameter at the given index for the target extension type.
+ *
+ * @see llvm::TargetExtType::getIntParameter()
+ */
+unsigned LLVMGetTargetExtTypeIntParam(LLVMTypeRef TargetExtTy, unsigned Idx);
+
 /**
  * @}
  */
diff --git a/llvm/lib/IR/Core.cpp b/llvm/lib/IR/Core.cpp
index 3b6b01fb78b0a..9ba7873106043 100644
--- a/llvm/lib/IR/Core.cpp
+++ b/llvm/lib/IR/Core.cpp
@@ -954,6 +954,32 @@ 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 LLVMGetTargetExtTypeNumTypeParams(LLVMTypeRef TargetExtTy) {
+  TargetExtType *Type = unwrap<TargetExtType>(TargetExtTy);
+  return Type->getNumTypeParameters();
+}
+
+LLVMTypeRef LLVMGetTargetExtTypeTypeParam(LLVMTypeRef TargetExtTy,
+                                          unsigned Idx) {
+  TargetExtType *Type = unwrap<TargetExtType>(TargetExtTy);
+  return wrap(Type->getTypeParameter(Idx));
+}
+
+unsigned LLVMGetTargetExtTypeNumIntParams(LLVMTypeRef TargetExtTy) {
+  TargetExtType *Type = unwrap<TargetExtType>(TargetExtTy);
+  return Type->getNumIntParameters();
+}
+
+unsigned LLVMGetTargetExtTypeIntParam(LLVMTypeRef TargetExtTy, unsigned Idx) {
+  TargetExtType *Type = unwrap<TargetExtType>(TargetExtTy);
+  return Type->getIntParameter(Idx);
+}
+
 /*===-- 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 bb5fae0dcd12e..dc6f2a9e7d206 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 518716168c423..fed8b1a697649 100644
--- a/llvm/tools/llvm-c-test/echo.cpp
+++ b/llvm/tools/llvm-c-test/echo.cpp
@@ -157,8 +157,26 @@ struct TypeCloner {
         return LLVMX86MMXTypeInContext(Ctx);
       case LLVMTokenTypeKind:
         return LLVMTokenTypeInContext(Ctx);
-      case LLVMTargetExtTypeKind:
-        assert(false && "Implement me");
+      case LLVMTargetExtTypeKind: {
+        const char *Name = LLVMGetTargetExtTypeName(Src);
+        unsigned NumTypeParams = LLVMGetTargetExtTypeNumTypeParams(Src);
+        unsigned NumIntParams = LLVMGetTargetExtTypeNumIntParams(Src);
+
+        SmallVector<LLVMTypeRef, 4> TypeParams((size_t)NumTypeParams);
+        SmallVector<unsigned, 4> IntParams((size_t)NumIntParams);
+
+        for (unsigned i = 0; i < TypeParams.size(); i++)
+          TypeParams[i] = Clone(LLVMGetTargetExtTypeTypeParam(Src, i));
+
+        for (unsigned i = 0; i < IntParams.size(); i++)
+          IntParams[i] = LLVMGetTargetExtTypeIntParam(Src, i);
+
+        LLVMTypeRef TargtExtTy = LLVMTargetExtTypeInContext(
+            Ctx, Name, TypeParams.data(), TypeParams.size(), IntParams.data(),
+            IntParams.size());
+
+        return TargtExtTy;
+      }
     }
 
     fprintf(stderr, "%d is not a supported typekind\n", Kind);

Accessors for the name, type parameters, and integer parameters are added. A
test is added to echo.ll
Copy link

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

@Benjins Benjins force-pushed the bns-target-ext-types-c-api-v2 branch from 3f6bb7e to 47628b2 Compare June 24, 2024 00:37
Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

Looks reasonable.

@@ -954,6 +954,32 @@ 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();
Copy link
Contributor

Choose a reason for hiding this comment

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

This looked iffy, but apparently StringSaver used to store the name guarantees null termination, so this should be safe.

return Type->getNumIntParameters();
}

unsigned LLVMGetTargetExtTypeIntParam(LLVMTypeRef TargetExtTy, unsigned Idx) {
Copy link
Contributor

Choose a reason for hiding this comment

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

A bit worried the type is going to change in the future, but I guess we'll have to deal with it if it happens.

for (unsigned i = 0; i < IntParams.size(); i++)
IntParams[i] = LLVMGetTargetExtTypeIntParam(Src, i);

LLVMTypeRef TargtExtTy = LLVMTargetExtTypeInContext(
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
LLVMTypeRef TargtExtTy = LLVMTargetExtTypeInContext(
LLVMTypeRef TargetExtTy = LLVMTargetExtTypeInContext(

Weird place to save a character :)

@nikic nikic requested a review from jcranmer-intel June 24, 2024 09:41
@nikic nikic merged commit f782ff8 into llvm:main Jun 26, 2024
8 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jun 26, 2024

LLVM Buildbot has detected a new failure on builder lldb-aarch64-windows running on linaro-armv8-windows-msvc-05 while building llvm at step 6 "test".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/141/builds/326

Here is the relevant piece of the build log for the reference:

Step 6 (test) failure: build (failure)
...
28.794 [2/5/283] Building CXX object tools\lldb\unittests\ValueObject\CMakeFiles\LLDBValueObjectTests.dir\DumpValueObjectOptionsTests.cpp.obj
29.285 [1/5/284] Linking CXX executable tools\lldb\unittests\UnwindAssembly\PPC64\PPC64InstEmulationTests.exe
llvm-lit.py: C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\utils\lit\lit\llvm\config.py:57: note: using lit tools: C:\Users\tcwg\scoop\apps\git\2.39.0.windows.2\usr\bin
llvm-lit.py: C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\utils\lit\lit\llvm\config.py:508: note: using clang: c:\users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\clang.exe
llvm-lit.py: C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\utils\lit\lit\llvm\config.py:508: note: using ld.lld: c:\users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\ld.lld.exe
llvm-lit.py: C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\utils\lit\lit\llvm\config.py:508: note: using lld-link: c:\users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\lld-link.exe
llvm-lit.py: C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\utils\lit\lit\llvm\config.py:508: note: using ld64.lld: c:\users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\ld64.lld.exe
llvm-lit.py: C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\utils\lit\lit\llvm\config.py:508: note: using wasm-ld: c:\users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\wasm-ld.exe
29.319 [1/4/-- Testing: 1976 tests, 2 workers --
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 
FAIL: lldb-shell :: SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp (1610 of 1976)
******************** TEST 'lldb-shell :: SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
# RUN: at line 19
c:\users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\clang.exe --target=specify-a-target-or-use-a-_host-substitution -target x86_64-pc-linux -gdwarf-5 -gsplit-dwarf    -fdebug-types-section -gpubnames -c C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\test\Shell\SymbolFile\DWARF\x86\dwp-foreign-type-units.cpp -o C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\test\Shell\SymbolFile\DWARF\x86\Output\dwp-foreign-type-units.cpp.tmp.main.o
# executed command: 'c:\users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\clang.exe' --target=specify-a-target-or-use-a-_host-substitution -target x86_64-pc-linux -gdwarf-5 -gsplit-dwarf -fdebug-types-section -gpubnames -c 'C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\test\Shell\SymbolFile\DWARF\x86\dwp-foreign-type-units.cpp' -o 'C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\test\Shell\SymbolFile\DWARF\x86\Output\dwp-foreign-type-units.cpp.tmp.main.o'
# note: command had no output on stdout or stderr
# RUN: at line 21
c:\users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\clang.exe --target=specify-a-target-or-use-a-_host-substitution -target x86_64-pc-linux -gdwarf-5 -gsplit-dwarf -DVARIANT    -fdebug-types-section -gpubnames -c C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\test\Shell\SymbolFile\DWARF\x86\dwp-foreign-type-units.cpp -o C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\test\Shell\SymbolFile\DWARF\x86\Output\dwp-foreign-type-units.cpp.tmp.foo.o
# executed command: 'c:\users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\clang.exe' --target=specify-a-target-or-use-a-_host-substitution -target x86_64-pc-linux -gdwarf-5 -gsplit-dwarf -DVARIANT -fdebug-types-section -gpubnames -c 'C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\test\Shell\SymbolFile\DWARF\x86\dwp-foreign-type-units.cpp' -o 'C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\test\Shell\SymbolFile\DWARF\x86\Output\dwp-foreign-type-units.cpp.tmp.foo.o'
# note: command had no output on stdout or stderr
# RUN: at line 23
c:\users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\ld.lld.exe C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\test\Shell\SymbolFile\DWARF\x86\Output\dwp-foreign-type-units.cpp.tmp.main.o C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\test\Shell\SymbolFile\DWARF\x86\Output\dwp-foreign-type-units.cpp.tmp.foo.o -o C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\test\Shell\SymbolFile\DWARF\x86\Output\dwp-foreign-type-units.cpp.tmp
# executed command: 'c:\users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\ld.lld.exe' 'C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\test\Shell\SymbolFile\DWARF\x86\Output\dwp-foreign-type-units.cpp.tmp.main.o' 'C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\test\Shell\SymbolFile\DWARF\x86\Output\dwp-foreign-type-units.cpp.tmp.foo.o' -o 'C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\test\Shell\SymbolFile\DWARF\x86\Output\dwp-foreign-type-units.cpp.tmp'
# .---command stderr------------
# | ld.lld: warning: cannot find entry symbol _start; not setting start address
# `-----------------------------
# RUN: at line 26
rm -f C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\test\Shell\SymbolFile\DWARF\x86\Output\dwp-foreign-type-units.cpp.tmp.dwp
# executed command: rm -f 'C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\test\Shell\SymbolFile\DWARF\x86\Output\dwp-foreign-type-units.cpp.tmp.dwp'
# note: command had no output on stdout or stderr
# RUN: at line 27
c:\users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\lldb.exe --no-lldbinit -S C:/Users/tcwg/llvm-worker/lldb-aarch64-windows/build/tools/lldb\test\Shell\lit-lldb-init-quiet    -o "type lookup IntegerType"    -o "type lookup FloatType"    -o "type lookup CustomType"    -b C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\test\Shell\SymbolFile\DWARF\x86\Output\dwp-foreign-type-units.cpp.tmp | c:\users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\filecheck.exe C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\test\Shell\SymbolFile\DWARF\x86\dwp-foreign-type-units.cpp --check-prefix=NODWP
# executed command: 'c:\users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\lldb.exe' --no-lldbinit -S 'C:/Users/tcwg/llvm-worker/lldb-aarch64-windows/build/tools/lldb\test\Shell\lit-lldb-init-quiet' -o 'type lookup IntegerType' -o 'type lookup FloatType' -o 'type lookup CustomType' -b 'C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\test\Shell\SymbolFile\DWARF\x86\Output\dwp-foreign-type-units.cpp.tmp'
# note: command had no output on stdout or stderr
# executed command: 'c:\users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\filecheck.exe' 'C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\test\Shell\SymbolFile\DWARF\x86\dwp-foreign-type-units.cpp' --check-prefix=NODWP
# .---command stderr------------
# | C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\test\Shell\SymbolFile\DWARF\x86\dwp-foreign-type-units.cpp:34:16: error: NODWP-NEXT: is not on the line after the previous match
# | // NODWP-NEXT: unsigned int
# |                ^
# | <stdin>:20:10: note: 'next' match was here
# |  typedef unsigned int IntegerType;
# |          ^
# | <stdin>:7:13: note: previous match ended here
# | unsigned int
# |             ^

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.

4 participants