[PATCH] D74588: Use DISABLE_LLVM_LINK_LLVM_DYLIB for TableGenTests

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added a comment.

In D74588#2657461 <https://reviews.llvm.org/D74588#2657461>, @beanz wrote:

> In D74588#2654289 <https://reviews.llvm.org/D74588#2654289>, @aaronpuchert wrote:
>
>> In D74588#2654185 <https://reviews.llvm.org/D74588#2654185>, @beanz wrote:
>>
>>> I think this bug is the other way around. LLVMTableGenGlobalISel should not be using `DISABLE_LLVM_LINK_LLVM_DYLIB`.
>>
>> The commit message of rL373651 <https://reviews.llvm.org/rL373651> says
>>
>>> Fixed the -DLLVM_LINK_LLVM_DYLIB=ON using DISABLE_LLVM_LINK_LLVM_DYLIB when creating the library. Apparently it automatically links to libLLVM.dylib and we don't want that from tablegen.
>
> This makes no sense. The tablegen tool build disables linking the dylib, which should be sufficient. We should never need to force it on a static archive. I’m 99% sure, that rL373651 <https://reviews.llvm.org/rL373651> is just wrong here, and that the solution to all of this is to just remove `DISABLE_LLVM_LINK_LLVM_DYLIB` from the library.

The buildbot logs are long gone now but the only reason I added it was because I had twenty-odd bots failing on the object library and this was the only way I could get both local builds and bot builds to work.

I'd hazard a guess it originated from from this in llvm/unittests/CMakeLists.txt

  # The target unittests may test APIs that aren't exported in libLLVM.so, so     
  # we need to always link against the static libraries.                          
  function(add_llvm_target_unittest test_dir_name)                                
    add_llvm_unittest(${test_dir_name} DISABLE_LLVM_LINK_LLVM_DYLIB ${ARGN})      
  endfunction()   

or at least has a similar issue but the only way to be sure is to remove it again and see what the bots complained about again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74588