[PATCH] D74588: Use DISABLE_LLVM_LINK_LLVM_DYLIB for TableGenTests

Daniel Sanders via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 30 13:07:50 PDT 2021


dsanders added a comment.

In D74588#2659067 <https://reviews.llvm.org/D74588#2659067>, @aaronpuchert wrote:

> In D74588#2658871 <https://reviews.llvm.org/D74588#2658871>, @beanz wrote:
>
>> Yep, and that's because of a different bug in how LLVMTableGenGlobalISel was setup.
>>
>> This patch would address the issues: [...]
>
> Ok, that allows me to remove the `DISABLE_LLVM_LINK_LLVM_DYLIB`, but
>
> - maybe it would be confusing to have an LLVM component in `llvm/utils` instead of `llvm/lib`, and
> - maybe this shouldn't be part of LLVM because it's only needed for `llvm-tblgen`? In other words maybe this isn't actually a component of LLVM proper but just a tool for building LLVM. If we make it a component library it will be part of `libLLVM.so`.

Yeah, I agree this doesn't belong in llvm/lib. They're llvm-tblgen sources.

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

> In D74588#2659067 <https://reviews.llvm.org/D74588#2659067>, @aaronpuchert wrote:
>
>> - maybe it would be confusing to have an LLVM component in `llvm/utils` instead of `llvm/lib`, and
>
> Yea, that is confusing. It is also confusing that `LLVMTableGenGlobalISel` was put into a static archive at all instead of just making it part of the tablegen executable like all the other tablegen backends and utils.
>
>> - maybe this shouldn't be part of LLVM because it's only needed for `llvm-tblgen`? In other words maybe this isn't actually a component of LLVM proper but just a tool for building LLVM. If we make it a component library it will be part of `libLLVM.so`.
>
> `LLVMTableGenGlobalISel` is trying to have its cake and eat it to. You can't make a library that you expect to behave like an LLVM component for linkages, but then not make it a component. `LLVMTableGen` is an LLVM component, even though it is really only used for building LLVM too, so there is prior art here.
>
> The alternative to making `LLVMTableGenGlobalISel` would be to hand roll the proper linkage behavior so that it doesn't pull in libLLVM.

All I wanted was a subdirectory to organize the sources specific to the GISel tablegen passes :-). I'd have been happy with GlobalISel/Foo.cpp in the llvm/utils/TableGen/CMakeLists.txt but CMake objects to that. I then went with the object library so that CMake would build the objects that I could then link as part of llvm-tblgen instead of being a proper library but the bots running older CMake's objected to that.

If there's a simple way to have sources for llvm-tblgen in a subdirectory of llvm/utils/TableGen then I'd be happy to switch to it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74588



More information about the llvm-commits mailing list