Add opaque pointers to the llvm-c API

[nico-abram]

Summary

This is based on https://reviews.llvm.org/D125168 which adds a wrapper to allow use of opaque pointers from the C API.

I added an opaque pointer mode test to echo.ll , and to fix assertions that forbid the use of mixed typed and opaque pointers that were triggering in it I had to also add wrappers for setOpaquePointers and supportsTypedPointers.

I also changed echo.ll to remove a bitcast i32* %x to i8*, because passing it through llvm-as and llvm-dis was generating a %0 = bitcast ptr %x to ptr, but when building that same bitcast in echo.cpp it was getting elided by IRBuilderBase::CreateCast (https://github.com/llvm/llvm-project/blob/08ac66124874d70dab63c731da0244f9e29ef168/llvm/include/llvm/IR/IRBuilder.h#L1998-L1999).

cc @lerno

[nikic]

llvm/include/llvm-c/Core.h

1453

nit: Missing period at end.

llvm/tools/llvm-c-test/echo.cpp

1004

You can use this unconditionally.

1037

Same here.

1395

Should be OpaquePointers for consistency?

Also, can we just change llvm_echo to accept the argument rather than adding two wrappers?

[nico-abram]

Addressed @nikic's comments, thanks for the feedback.

[nikic]

llvm/include/llvm-c/Core.h

556

I'm a bit uncertain about this API. The supportsTypedPointers() naming dates back to a time where we supported mixed typed and opaque pointers, so "supports opaque pointers" and "not supports typed pointers" were not equivalent. There's a couple of different possibilities here:

• LLVMContextSupportsTypedPointers() as proposed.
• LLVMContextHasOpaquePointers() the inverse, which mirrors LLVMContextSetOpaquePointers() a bit more nicely.
• Expose LLVMPointerTypeIsOpaque() or similar instead, which would work on a pointer type rather than context.

The latter is slightly less general (you can only use it if you already have a pointer type), but it's the one that we actually use most often in practice. We only use supportsTypedPointers() for automatic switching into opaque pointer mode during LL/BC parsing.

I'd personally lean towards exposing LLVMPointerTypeIsOpaque().

llvm/tools/llvm-c-test/echo.cpp

1034

nit: Combine declaration and assignment.

llvm/tools/llvm-c-test/main.c

52

Make it --echo --opaque-pointers for the sake of consistency?

[nico-abram]

llvm/tools/llvm-c-test/main.c

52

I used -opaque-pointers since that's what the tools use, like llvm-as and llvm-dis, as documented here

If keeping it consistent with the rest of llvm-c-test is more important I can change it

[nikic]

llvm/tools/llvm-c-test/main.c

52

The preferred option format is tool-specific -- opt etc do accept options in --opaque-pointers format as well, but for historical reason we tend to use the single-dash spelling for them.

[aeubanks]

llvm/include/llvm-c/Core.h

556

+1

[nico-abram]

Thanks for the feedback.

I uploaded a new diff, removed LLVMContextSupportsTypedPointers and added LLVMPointerTypeIsOpaque, also changed the echo.cpp flag to use the -- style.

[nikic]

LGTM

[nico-abram]

I don't have commit rights, could somebody please land this for me?

llvm/include/llvm-c/Core.h

  * If true, only the names of GlobalValue objects will be available in the IR.
  * This can be used to save memory and runtime, especially in release mode.
  *
  * @see LLVMContext::setDiscardValueNames()
  */
 void LLVMContextSetDiscardValueNames(LLVMContextRef C, LLVMBool Discard);
 
 /**
+ * Set whether the given context is in opaque pointer mode.
+ *
+ * @see LLVMContext::setOpaquePointers()
+ */
+void LLVMContextSetOpaquePointers(LLVMContextRef C, LLVMBool OpaquePointers);
+
+/**
  * Destroy a context instance.
  *
  * This should be called for every call to LLVMContextCreate() or memory
  * will be leaked.
  */
 void LLVMContextDispose(LLVMContextRef C);
 
 /**

  * The created type will exist in the context that its pointee type
  * exists in.
  *
  * @see llvm::PointerType::get()
  */
 LLVMTypeRef LLVMPointerType(LLVMTypeRef ElementType, unsigned AddressSpace);
 
 /**
+ * Determine whether a pointer is opaque.
+ *
+ * True if this is an instance of an opaque PointerType.
+ *
+ * @see llvm::Type::isOpaquePointerTy()
+ */
+LLVMBool LLVMPointerTypeIsOpaque(LLVMTypeRef Ty);
+
+/**
+ * Create an opaque pointer type in a context.
+ *
+ * @see llvm::PointerType::get()
+ */
+LLVMTypeRef LLVMPointerTypeInContext(LLVMContextRef C, unsigned AddressSpace);
+
+/**
  * Obtain the address space of a pointer type.
  *
  * This only works on types that represent pointers.
  *
  * @see llvm::PointerType::getAddressSpace()
  */
 unsigned LLVMGetPointerAddressSpace(LLVMTypeRef PointerTy);
 

llvm/lib/IR/Core.cpp

 LLVMBool LLVMContextShouldDiscardValueNames(LLVMContextRef C) {
   return unwrap(C)->shouldDiscardValueNames();
 }
 
 void LLVMContextSetDiscardValueNames(LLVMContextRef C, LLVMBool Discard) {
   unwrap(C)->setDiscardValueNames(Discard);
 }
 
+void LLVMContextSetOpaquePointers(LLVMContextRef C, LLVMBool OpaquePointers) {
+  unwrap(C)->setOpaquePointers(OpaquePointers);
+}
+
 void LLVMContextDispose(LLVMContextRef C) {
   delete unwrap(C);
 }
 
 unsigned LLVMGetMDKindIDInContext(LLVMContextRef C, const char *Name,
                                   unsigned SLen) {
   return unwrap(C)->getMDKindID(StringRef(Name, SLen));
 }

 LLVMTypeRef LLVMArrayType(LLVMTypeRef ElementType, unsigned ElementCount) {
   return wrap(ArrayType::get(unwrap(ElementType), ElementCount));
 }
 
 LLVMTypeRef LLVMPointerType(LLVMTypeRef ElementType, unsigned AddressSpace) {
   return wrap(PointerType::get(unwrap(ElementType), AddressSpace));
 }
 
+LLVMBool LLVMPointerTypeIsOpaque(LLVMTypeRef Ty) {
+  return unwrap(Ty)->isOpaquePointerTy();
+}
+
 LLVMTypeRef LLVMVectorType(LLVMTypeRef ElementType, unsigned ElementCount) {
   return wrap(FixedVectorType::get(unwrap(ElementType), ElementCount));
 }
 
 LLVMTypeRef LLVMScalableVectorType(LLVMTypeRef ElementType,
                                    unsigned ElementCount) {
   return wrap(ScalableVectorType::get(unwrap(ElementType), ElementCount));
 }

 }
 
 unsigned LLVMGetVectorSize(LLVMTypeRef VectorTy) {
   return unwrap<VectorType>(VectorTy)->getElementCount().getKnownMinValue();
 }
 
 /*--.. Operations on other types ...........................................--*/
 
+LLVMTypeRef LLVMPointerTypeInContext(LLVMContextRef C, unsigned AddressSpace) {
+  return wrap(PointerType::get(*unwrap(C), AddressSpace));
+}
+
 LLVMTypeRef LLVMVoidTypeInContext(LLVMContextRef C)  {
   return wrap(Type::getVoidTy(*unwrap(C)));
 }
 LLVMTypeRef LLVMLabelTypeInContext(LLVMContextRef C) {
   return wrap(Type::getLabelTy(*unwrap(C)));
 }
 LLVMTypeRef LLVMTokenTypeInContext(LLVMContextRef C) {
   return wrap(Type::getTokenTy(*unwrap(C)));

llvm/test/Bindings/llvm-c/echo.ll

 ; RUN: llvm-as < %s | llvm-dis > %t.orig
 ; RUN: llvm-as < %s | llvm-c-test --echo > %t.echo
 ; RUN: diff -w %t.orig %t.echo
+;
+; RUN: llvm-as -opaque-pointers < %s | llvm-dis -opaque-pointers > %t.orig_opaque
+; RUN: llvm-as -opaque-pointers < %s | llvm-c-test --echo --opaque-pointers > %t.echo_opaque
+; RUN: diff -w %t.orig_opaque %t.echo_opaque
 
 source_filename = "/test/Bindings/echo.ll"
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.11.0"
 
 module asm "classical GAS"
 
 %S = type { i64, %S* }

 declare i8* @llvm.stacksave()
 declare void @llvm.stackrestore(i8*)
 declare void @llvm.lifetime.start.p0i8(i64, i8*)
 declare void @llvm.lifetime.end.p0i8(i64, i8*)
 
 define void @test_intrinsics() {
 entry:
   %sp = call i8* @llvm.stacksave()
-  %x = alloca i32, align 4
-  %0 = bitcast i32* %x to i8*
-  call void @llvm.lifetime.start.p0i8(i64 4, i8* %0)
-  call void @llvm.lifetime.end.p0i8(i64 4, i8* %0)
+  %0 = alloca i8, align 1
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* %0)
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* %0)
   call void @llvm.stackrestore(i8* %sp)
   ret void
 }
 
 !llvm.dbg.cu = !{!0, !2}
 !llvm.module.flags = !{!3}
 
 !0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)

llvm/tools/llvm-c-test/echo.cpp

switch (Kind) {
         return S;
       }
       case LLVMArrayTypeKind:
         return LLVMArrayType(
           Clone(LLVMGetElementType(Src)),
           LLVMGetArrayLength(Src)
         );
       case LLVMPointerTypeKind:
-        return LLVMPointerType(
-          Clone(LLVMGetElementType(Src)),
-          LLVMGetPointerAddressSpace(Src)
-        );
+        if (LLVMPointerTypeIsOpaque(Src))
+          return LLVMPointerTypeInContext(Ctx, LLVMGetPointerAddressSpace(Src));
+        else
+          return LLVMPointerType(Clone(LLVMGetElementType(Src)),
+                                 LLVMGetPointerAddressSpace(Src));
       case LLVMVectorTypeKind:
         return LLVMVectorType(
           Clone(LLVMGetElementType(Src)),
           LLVMGetVectorSize(Src)
         );
       case LLVMScalableVectorTypeKind:
         return LLVMScalableVectorType(Clone(LLVMGetElementType(Src)),
                                       LLVMGetVectorSize(Src));
if (!Begin) {
     goto FunDecl;
   }
 
   while (true) {
     size_t NameLen;
     const char *Name = LLVMGetValueName2(Cur, &NameLen);
     if (LLVMGetNamedGlobal(M, Name))
       report_fatal_error("GlobalVariable already cloned");
-    LLVMAddGlobal(M, LLVMGetElementType(TypeCloner(M).Clone(Cur)), Name);
+    LLVMAddGlobal(M, TypeCloner(M).Clone(LLVMGlobalGetValueType(Cur)), Name);
 
     Next = LLVMGetNextGlobal(Cur);
     if (Next == nullptr) {
       if (Cur != End)
         report_fatal_error("");
       break;
     }
 
     LLVMValueRef Prev = LLVMGetPreviousGlobal(Next);
     if (Prev != Cur)
       report_fatal_error("Next.Previous global is not Current");
FunDecl:
 
   Cur = Begin;
   Next = nullptr;
   while (true) {
     size_t NameLen;
     const char *Name = LLVMGetValueName2(Cur, &NameLen);
     if (LLVMGetNamedFunction(M, Name))
       report_fatal_error("Function already cloned");
-    auto Ty = LLVMGetElementType(TypeCloner(M).Clone(Cur));
+    LLVMTypeRef Ty = TypeCloner(M).Clone(LLVMGlobalGetValueType(Cur));
+
     auto F = LLVMAddFunction(M, Name, Ty);
 
     // Copy attributes
     for (int i = LLVMAttributeFunctionIndex, c = LLVMCountParams(F);
          i <= c; ++i) {
       for (unsigned k = 0, e = LLVMGetLastEnumAttributeKind(); k < e; ++k) {
         if (auto SrcA = LLVMGetEnumAttributeAtIndex(Cur, i, k)) {
           auto Val = LLVMGetEnumAttributeValue(SrcA);
           auto DstA = LLVMCreateEnumAttribute(Ctx, k, Val);
           LLVMAddAttributeAtIndex(F, i, DstA);
         }
while (true) {
     LLVMNamedMDNodeRef PrevMD = LLVMGetPreviousNamedMetadata(NextMD);
     if (PrevMD != CurMD)
       report_fatal_error("Next.Previous Named MD Node is not Current");
 
     CurMD = NextMD;
   }
 }
 
-int llvm_echo(void) {
+int llvm_echo(bool OpaquePointers) {
   LLVMEnablePrettyStackTrace();
 
   LLVMModuleRef Src = llvm_load_module(false, true);
   size_t SourceFileLen;
   const char *SourceFileName = LLVMGetSourceFileName(Src, &SourceFileLen);
   size_t ModuleIdentLen;
   const char *ModuleName = LLVMGetModuleIdentifier(Src, &ModuleIdentLen);
   LLVMContextRef Ctx = LLVMContextCreate();
+  if (OpaquePointers)
+    LLVMContextSetOpaquePointers(Ctx, true);
   LLVMModuleRef M = LLVMModuleCreateWithNameInContext(ModuleName, Ctx);
 
   LLVMSetSourceFileName(M, SourceFileName, SourceFileLen);
   LLVMSetModuleIdentifier(M, ModuleName, ModuleIdentLen);
 
   LLVMSetTarget(M, LLVMGetTarget(Src));
   LLVMSetModuleDataLayout(M, LLVMGetModuleDataLayout(Src));
   if (strcmp(LLVMGetDataLayoutStr(M), LLVMGetDataLayoutStr(Src)))

llvm/tools/llvm-c-test/llvm-c-test.h

 // object.c
 int llvm_object_list_sections(void);
 int llvm_object_list_symbols(void);
 
 // targets.c
 int llvm_targets_list(void);
 
 // echo.c
-int llvm_echo(void);
+int llvm_echo(bool OpaquePointers);
 
 // diagnostic.c
 int llvm_test_diagnostic_handler(void);
 
 // attributes.c
 int llvm_test_function_attributes(void);
 int llvm_test_callsite_attributes(void);
 
 #ifdef __cplusplus
 }
 #endif /* !defined(__cplusplus) */
 
 #endif

llvm/tools/llvm-c-test/main.c

static void print_usage(void) {
   fprintf(stderr, "    Read lines of triple, hex ascii machine code from stdin "
                   "- print disassembly\n\n");
   fprintf(stderr, "  * --calc\n");
   fprintf(
       stderr,
       "    Read lines of name, rpn from stdin - print generated module\n\n");
   fprintf(stderr, "  * --echo\n");
   fprintf(stderr, "    Read bitcode file from stdin - print it back out\n\n");
+  fprintf(stderr, "  * --echo --opaque-pointers\n");
+  fprintf(stderr, "    Read bitcode file from stdin - print it back out in "
+                  "opaque pointer mode\n\n");
   fprintf(stderr, "  * --test-diagnostic-handler\n");
   fprintf(stderr,
           "    Read bitcode file from stdin with a diagnostic handler set\n\n");
   fprintf(stderr, "  * --test-dibuilder\n");
   fprintf(stderr,
           "    Run tests for the DIBuilder C API - print generated module\n\n");
 }
 
int main(int argc, char **argv) {
   } else if (argc == 2 && !strcmp(argv[1], "--add-named-metadata-operand")) {
     return llvm_add_named_metadata_operand();
   } else if (argc == 2 && !strcmp(argv[1], "--set-metadata")) {
     return llvm_set_metadata();
   } 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")) {
     return llvm_test_callsite_attributes();
-  } else if (argc == 2 && !strcmp(argv[1], "--echo")) {
-    return llvm_echo();
+  } else if ((argc == 2 || argc == 3) && !strcmp(argv[1], "--echo")) {
+    return llvm_echo(argc == 3 ? !strcmp(argv[2], "--opaque-pointers") : 0);
   } else if (argc == 2 && !strcmp(argv[1], "--test-diagnostic-handler")) {
     return llvm_test_diagnostic_handler();
   } else if (argc == 2 && !strcmp(argv[1], "--test-dibuilder")) {
     return llvm_test_dibuilder();
   } else {
     print_usage();
   }
 
   return 1;
 }