-
Notifications
You must be signed in to change notification settings - Fork 14.1k
[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
base: main
Are you sure you want to change the base?
Conversation
4c219aa
to
29dbd9e
Compare
#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 |
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.
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.
29dbd9e
to
d478b2b
Compare
d478b2b
to
1cf4a9d
Compare
/// 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) || \ |
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.
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.
llvm/include/llvm-c/Remarks.h
Outdated
@@ -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); |
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.
I think that this should remain part of the ABI.
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.
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.
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 newLLVM_C_ABI
annotation, which behaves like theLLVM_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
llvm-c/Visibility.h
header file that defines a newLLVM_C_ABI
macro. The macro resolves to the proper DLL export/import annotation on Windows and a "default" visibility annotation elsewhere.LLVM_C_ABI
definition fromllvm/Support/Compiler.h
. Update the small number ofLLVM_C_ABI
references to get it from the newllvm-c/Visibility.h
header.llvm-c
interface using the Interface Definition Scanner (IDS) tool.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: