[PATCH] D54439: CMake: Make most target symbols hidden by default

Wed Jul 10 09:23:16 PDT 2019

tstellar marked an inline comment as done.
tstellar added inline comments.
Herald added subscribers: wuzish, MaskRay.

================
Comment at: llvm/include/llvm/PassSupport.h:62-63
   static llvm::once_flag Initialize##passName##PassFlag;                       \
+  LLVM_EXTERNAL_VISIBILITY /* FIXME: Workaround to avoid ld warnings on OS X*/ \
+			   /* when this is compiled with -fvisibility=hidden*/ \
   void llvm::initialize##passName##Pass(PassRegistry &Registry) {              \
----------------
rnk wrote:
> tstellar wrote:
> > rnk wrote:
> > > Is this a speculative fix, or were you able to reproduce the problem and confirm this fixes it?
> > This is speculative.   I don't have an OS X system to test on.  I have tried to reproduce on linux with clang-7, but I don't see any object file differences in files mentioned in the warnings, like X86Targetmachine.cpp.o.
> > 
> > It seems like this warning is triggered when you have a cpp file that uses one of the INITIALIZE_PASS macros and also calls the initialize*Pass function in the same file.   My best guess is this is related to inlining decisions in some way, but I still don't know the root cause.
> @echristo, do you want to apply this locally and try to reproduce, since you reported the issue last time around?
> 
> @tstellar My worry is that maybe this warning is triggered more broadly by any templates instantiated and called by hidden and default visibility functions. I'd like to nail down when it happens, or disable it entirely, since I'm positive we don't care about being able to override C++ template functions.
Ok for reference, similar errors were reported in this bug: https://llvm.org/PR31782.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54439/new/

https://reviews.llvm.org/D54439