[llvm] compile out LLVM_ABI unless building as a shared lib or dylib

[andrurogerz]

Purpose

Ensure that LLVM_ABI and related macros are compiled out when not building LLVM as a shared library.

Overview

I observed that by default on Linux and Darwin, the LLVM_ABI macro was being defined to a visibility annotation even when not building as a shared library. This is contrary to my expectation: I believe LLVM_ABI and related macros should compile to nothing unless building a DLL for Windows or shared object file for other targets.

The root cause appears to be that LLVM_ENABLE_PLUGINS it set to on by default. While that's a reasonable default, I don't think it actually requires the LLVM_ABI macros modify symbol visibility.

Background

This change is in support of the project to annotate LLVM's public interfaces described here. As we begin adding annotations throughout the codebase, it will be much safer to have the annotations be inactive by default and enable them once the effort is complete.

Validation

cmake -S llvm -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS="llvm;clang" -DLLVM_OPTIMIZED_TABLEGEN=ON
ninja -C build check-llvm

[andrurogerz]

++ @compnerd , @vgvassilev

@fsfod do you have more context for why the LLVM_ABI macros are always defined when LLVM_ENABLE_PLUGINS is On? It didn't make sense to me, but there's a good chance I'm not understanding something.

I feel strongly that we want the LLVM_ABI macros to be inactive by default to reduce risk as I annotate the codebase. But there's a good chance this is not the correct fix.

[llvmbot]

@llvm/pr-subscribers-llvm-support

Author: Andrew Rogers (andrurogerz)

Changes

Purpose

Ensure that LLVM_ABI and related macros are compiled out when not building LLVM as a shared library.

Overview

I observed that by default on Linux and Darwin, the LLVM_ABI macro was being defined to a visibility annotation even when not building as a shared library. This is contrary to my expectation: I believe LLVM_ABI and related macros should compile to nothing unless building a DLL for Windows or shared object file for other targets.

The root cause appears to be that LLVM_ENABLE_PLUGINS it set to on by default. While that's a reasonable default, I don't think it actually requires the LLVM_ABI macros modify symbol visibility.

Background

This change is in support of the project to annotate LLVM's public interfaces described here. As we begin adding annotations throughout the codebase, it will be much safer to have the annotations be inactive by default and enable them once the effort is complete.

Validation

cmake -S llvm -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS="llvm;clang" -DLLVM_OPTIMIZED_TABLEGEN=ON
ninja -C build check-llvm

---

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

1 Files Affected:

• (modified) llvm/include/llvm/Support/Compiler.h (+1-2)

diff --git a/llvm/include/llvm/Support/Compiler.h b/llvm/include/llvm/Support/Compiler.h
index d3772896069cc..2a4cbe673d35e 100644
--- a/llvm/include/llvm/Support/Compiler.h
+++ b/llvm/include/llvm/Support/Compiler.h
@@ -175,8 +175,7 @@
 // Marker to add to classes or functions in public headers that should not have
 // export macros added to them by the clang tool
 #define LLVM_ABI_NOT_EXPORTED
-#if defined(LLVM_BUILD_LLVM_DYLIB) || defined(LLVM_BUILD_SHARED_LIBS) ||       \
-    defined(LLVM_ENABLE_PLUGINS)
+#if defined(LLVM_BUILD_LLVM_DYLIB) || defined(LLVM_BUILD_SHARED_LIBS)
 // Some libraries like those for tablegen are linked in to tools that used
 // in the build so can't depend on the llvm shared library. If export macros
 // were left enabled when building these we would get duplicate or

[fsfod]

I think i was trying to preserve that symbols for static builds are exported from executables so plugins can us them on Linux. I think i had already converted some existing visibility macro use over to LLVM_ABI.