[flang] Fix building against clang dylib

[awarzynski]

Hey @mgorny , thanks for working on this

I thought that this build configuration is actually tested by flang-aarch64-dylib, so I'm not sure why it fails for you. I suspect that you only have libclang.dylib available on your system? And the buildbot would still build libClangBasic.so/a as a separate library?

flang/lib/FrontendTool/CMakeLists.txt

18

Where is TARGET defined? Please bear with me - I'm confused that in Flang (which only defines Flang CMake targets) we would be checking whether "target" is a particular Clang target?

[mgorny]

In D136606#3879466, @awarzynski wrote:

Hey @mgorny , thanks for working on this

I thought that this build configuration is actually tested by flang-aarch64-dylib, so I'm not sure why it fails for you. I suspect that you only have libclang.dylib available on your system? And the buildbot would still build libClangBasic.so/a as a separate library?

Yes, we don't install static clang libraries on Gentoo, and the buildbot probably has them (since the targets are present when doing in-tree build).

flang/lib/FrontendTool/CMakeLists.txt

18

Unless I'm mistaken, if you're doing in-tree build of LLVM + Clang + Flang, then targets from other projects should be visible here. FWICS LLVM is ensuring that Clang is processed prior to Flang in all cases, so it should work.

[awarzynski]

flang/lib/FrontendTool/CMakeLists.txt

18

Sorry, I was not familiar with if (TARGET <target>), hence my question. For future reference, it's documented under CMake's existance checks.

This makes sense, but I'm thinking that the following would be a bit more idiomatic. I'm also thinking that perhaps libclang-cpp should be added as a dependency too.

if(CLANG_LINK_CLANG_DYLIB)
 add_dependencies(flangFrontendTool clangBasic)
else()
  add_dependencies(flangFrontendTool clang-cpp)
endif()

CLANG_LINK_CLANG_DYLIB is defined whenever LLVM_LINK_LLVM_DYLIB is. So that ^^^ should be safe and always correct. WDYT?

[mgorny]

flang/lib/FrontendTool/CMakeLists.txt

18

I suppose you meant if the other way around but yes, it should work. I'm going to try it right now and update the patch in a minute.

[mgorny]

Update to use CLANG_LINK_CLANG_DYLIB for the dependency, and to add a dep on clang-cpp alternatively to clangBasic.

[awarzynski]

Thanks for the updates and apologies for the delay, LGTM!

[MaskRay]

Which build does this patch fix? My -DLLVM_ENABLE_PROJECTS='llvm;clang;mlir;flang' -DLLVM_LINK_LLVM_DYLIB=on build ninja check-flang works without this patch.

Note also that lldb has some clangBasic uses without the clang-cpp dispatch.

[mgorny]

In D136606#3890328, @MaskRay wrote:

Which build does this patch fix? My -DLLVM_ENABLE_PROJECTS='llvm;clang;mlir;flang' -DLLVM_LINK_LLVM_DYLIB=on build ninja check-flang works without this patch.

Note also that lldb has some clangBasic uses without the clang-cpp dispatch.

-DCLANG_LINK_CLANG_DYLIB=ON when no static libs are installed.

[mgorny]

In D136606#3889611, @awarzynski wrote:

Thanks for the updates and apologies for the delay, LGTM!

No problem. Thanks!

[awarzynski]

In D136606#3890328, @MaskRay wrote:

Which build does this patch fix? My -DLLVM_ENABLE_PROJECTS='llvm;clang;mlir;flang' -DLLVM_LINK_LLVM_DYLIB=on build ninja check-flang works without this patch.

Note also that lldb has some clangBasic uses without the clang-cpp dispatch.

Ah, we should've looked at LLDB in search for inspiration - they do indeed have a nice solution :) I think that the approach in this patch is fine and we can improve things later (i.e. there's no rush). I created https://github.com/llvm/llvm-project/issues/58663 to track this. Thanks for the pointer @MaskRay !

flang/lib/Frontend/CMakeLists.txt

 get_property(dialect_libs GLOBAL PROPERTY MLIR_DIALECT_LIBS)
 
 add_flang_library(flangFrontend
   CompilerInstance.cpp
   CompilerInvocation.cpp
   CodeGenOptions.cpp
   FrontendAction.cpp
   FrontendActions.cpp
   FrontendOptions.cpp
   TextDiagnosticPrinter.cpp
   TextDiagnosticBuffer.cpp
   TextDiagnostic.cpp
 
   DEPENDS
-  clangBasic
   FIRBuilder
   FIRDialect
   FIRSupport
   FIROptTransformsPassIncGen
   HLFIRDialect
   MLIRIR
   ${dialect_libs}
 
   LINK_LIBS
   FortranParser
   FortranSemantics
   FortranEvaluate
   FortranCommon
   FortranLower
-  clangBasic
-  clangDriver
   FIRDialect
   FIRSupport
   FIRBuilder
   FIRCodeGen
   FIRTransforms
   HLFIRDialect
   MLIRTransforms
   MLIRLLVMToLLVMIRTranslation
   MLIRSCFToControlFlow
   ${dialect_libs}
 
   LINK_COMPONENTS
   Passes
   Analysis
   IRReader
   Option
   Support
   Target
   FrontendOpenACC
   FrontendOpenMP
 )
+
+if(CLANG_LINK_CLANG_DYLIB)
+  add_dependencies(flangFrontend clang-cpp)
+else()
+  add_dependencies(flangFrontend clangBasic)
+endif()
+
+clang_target_link_libraries(flangFrontend
+  PRIVATE
+  clangBasic
+  clangDriver
+)

flang/lib/FrontendTool/CMakeLists.txt

 add_flang_library(flangFrontendTool
   ExecuteCompilerInvocation.cpp
 
   DEPENDS
   # This makes sure that the MLIR dependencies of flangFrontend (which are
   # transitively required here) are generated before this target is build.
   flangFrontend
-  clangBasic
 
   LINK_LIBS
   flangFrontend
-  clangBasic
-  clangDriver
   MLIRPass
 
   LINK_COMPONENTS
   Option
   Support
 )
+
+if(CLANG_LINK_CLANG_DYLIB)
+  add_dependencies(flangFrontend clang-cpp)
+else()
+  add_dependencies(flangFrontendTool clangBasic)
+endif()
+
+clang_target_link_libraries(flangFrontendTool
+  PRIVATE
+  clangBasic
+  clangDriver
+)