Always include LLVM CMake modules.

[bjacob]

These LLVM CMake modules used to be unconditionally included, but #2430 changed that to only conditionally including them in the standalone and embedded builds, leaving out the external-project build.

That broke integration in https://github.com/iree-org/iree where Stablehlo is an external project. Error log here. These are linking errors when building with Clang or GCC, with unresolved symbols for typeinfo for various types, referenced from Stablehlo symbols. typeinfo is a RTTI feature, and all LLVM-derived projects normally disable RTTI, which is why these symbols are not defined, and why that is normally not a problem. What changed with #2430 was that suddenly, the disabling of RTTI was itself disabled in our external build of Stablehlo. Here is the code that is responsible for disabling RTTI. It was not kicking in anymore, because LLVM_COMPILER_IS_GCC_COMPATIBLE was not defined. It is normally defined by this include just above in that file.

But as of #2430, the include(AddLLVM) itself was no longer done in the Stablehlo project, instead the Stablehlo external build was relying on include(AddLLVM) to have been done in the parent project, which it was. The problem was that while the parent-project include(AddLLVM) did define all the functions provided by that module, the CMake variable definitions such as LLVM_COMPILER_IS_GCC_COMPATIBLE were made on the parent project and were not visible to the child project.

Summary of the above: in their current shape, LLVM CMake modules such as AddLLVM (and AddMLIR, which Stablehlo uses, and which itself relies on AddLLVM) must be included by any child project that uses them, even if the parent project already included them.

[bjacob]

@GleasonK @ScottTodd @jpienaar

[GleasonK]

Hmm.. I think we did this was some issue with calling include(HandleLLVMOptions), maybe it was in the torch-mlir project's use of StableHLO (cc @qingyunqu?)

We're mostly bazel internally so if this works for IREE / other cmake users of StableHLO and it passes our CMake CI then LGTM.

Do you need this in ASAP or can we give a day or two to allow someone to confirm that this works in torch-mlir as well?

[bjacob]

I see, thanks for bringing that up. No urgency, as we can live with a cherry-pick for the time it takes to resolve this. Happy to find a common solution with torch-mlir (which IREE also uses as a submodule).

[bjacob]

Also, if there's an issue specifically with include(HandleLLVMOptions), that's actually not proximate to the errors I ran into. For the lines being moved here, only include(AddLLVM) is directly solving my issue.

[ScottTodd]

Happy to find a common solution with torch-mlir (which IREE also uses as a submodule).

FWIW, IREE's usage of torch-mlir has a custom CMake file that I think doesn't pull in stablehlo: https://github.com/iree-org/iree/blob/main/compiler/plugins/input/Torch/torch-mlir/CMakeLists.txt

[bjacob]

@GleasonK , @qingyunqu , I have updated this PR to leave HandleLLVMOptions unchanges (still only included in the standalone and embedded builds, not external). Looking at what it does, I agree that it is unwanted in the external build. WDYT ?

[GleasonK]

I think this should be good. Thanks!

[bjacob]

Thanks!