Skip to content

[llvm] annotate interfaces in llvm-c for DLL export #141701

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

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

andrurogerz
Copy link
Contributor

Purpose

This patch is one in a series of code-mods that annotate LLVM’s public interface for export. This patch annotates the llvm-c interface with a new LLVM_C_ABI annotation, which behaves like the LLVM_ABI. This annotation currently has no meaningful impact on the LLVM build; however, it is a prerequisite to support an LLVM Windows DLL (shared library) build.

Overview

  1. Add a new llvm-c/Visibility.h header file that defines a new LLVM_C_ABI macro. The macro resolves to the proper DLL export/import annotation on Windows and a "default" visibility annotation elsewhere.
  2. Remove the existing LLVM_C_ABI definition from llvm/Support/Compiler.h. Update the small number of LLVM_C_ABI references to get it from the new llvm-c/Visibility.h header.
  3. Code-mod annotate the public llvm-c interface using the Interface Definition Scanner (IDS) tool.
  4. Format the changes with clang-format.

Background

This effort is tracked in #109483. Additional context is provided in this discourse, and documentation for LLVM_ABI and related annotations is found in the LLVM repo here.

Validation

Local builds and tests to validate cross-platform compatibility. This included llvm, clang, and lldb on the following configurations:

  • Windows with MSVC
  • Windows with Clang
  • Linux with GCC
  • Linux with Clang
  • Darwin with Clang

@andrurogerz andrurogerz force-pushed the llvmdll-lib-llvm-c branch 2 times, most recently from 4c219aa to 29dbd9e Compare May 28, 2025 15:33
Comment on lines +21 to +40
#if defined(LLVM_BUILD_LLVM_DYLIB) || defined(LLVM_BUILD_SHARED_LIBS) || \
defined(LLVM_ENABLE_PLUGINS)
#if defined(LLVM_BUILD_STATIC)
#define LLVM_C_ABI
#elif defined(_WIN32) && !defined(__MINGW32__)
#if defined(LLVM_EXPORTS)
#define LLVM_C_ABI __declspec(dllexport)
#else
#define LLVM_C_ABI __declspec(dllimport)
#endif
#elif defined(__has_attribute) && __has_attribute(visibility)
#define LLVM_C_ABI __attribute__((visibility("default")))
#else
#define LLVM_C_ABI
#endif
#else
#define LLVM_C_ABI
#endif
Copy link
Member

Choose a reason for hiding this comment

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

I really would like to see this be simplified for LLVM-C, perhaps something like:

#if defined(LLVM_C_STATIC)
# define LLVM_C_ABI
#else
# if defined(_WIN32) && !defined(__MINGW32__)
#   if defined(LLVM_C_EXPORTS)
#     define LLVM_C_ABI __declspec(dllexport)
#   else
#     define LLVM_C_ABI __declspec(dllimport)
#   endif
# else
#   if defined(__has_attribute) && __has_attribute(visibility)
#     define LLVM_C_ABI __attribute__((__visibility__("default")))
#   else
#     define LLVM_C_ABI
#   endif
# endif
#endif

This way we can independently control whether LLVM-C is static or dynamic.

@andrurogerz andrurogerz force-pushed the llvmdll-lib-llvm-c branch from 29dbd9e to d478b2b Compare June 3, 2025 23:20
@andrurogerz andrurogerz force-pushed the llvmdll-lib-llvm-c branch from d478b2b to 1cf4a9d Compare June 4, 2025 14:07
/// LLVM_C_ABI is the export/visibility macro used to mark symbols declared in
/// llvm-c as exported when llvm is built as a shared library.

#if defined(LLVM_BUILD_LLVM_DYLIB) || defined(LLVM_BUILD_SHARED_LIBS) || \
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that we need to bind this to the LLVM's build style. That is, it should be safe to use a shared llvmc and a static LLVM.

@@ -334,7 +334,7 @@ LLVM_C_ABI extern void LLVMRemarkParserDispose(LLVMRemarkParserRef Parser);
*
* \since REMARKS_API_VERSION=0
*/
LLVM_C_ABI extern uint32_t LLVMRemarkVersion(void);
extern uint32_t LLVMRemarkVersion(void);
Copy link
Member

Choose a reason for hiding this comment

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

I think that this should remain part of the ABI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one is weird. LLVMRemarkVersion is implemented in a different library here. I don't know what the correct solution is here, but the implementation is not linked into llvm so can't be exported by llvm. It feels like either the header doesn't belong here or the implementation should be moved so it does link into llvm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants