awarzynski added inline comments.
================ Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:934-936 + // Default to the <driver-path>/../lib and + // <driver-path>/../runtimes/runtimes-bins/lib directories. This works fine + // on the platforms that we have tested so far. We will probably have to ---------------- pscoro wrote: > awarzynski wrote: > > Am I correct thinking that: > > * "<driver-path>/../lib" is for `Fortran_main.a`, and > > * "<driver-path>/../runtimes/runtimes-bins/lib " is for `libflang-rt`? > > > > Could you document this? Thanks! > Yep thats right, documentation added > Yep thats right, documentation added Could you document _where_ these libraries are located? Also, could you add some relevant comments here? For example: ``` // Add search path for XYZ llvm::sys::path::append(BuildLibPath, "lib"); ``` ================ Comment at: flang/docs/Flang-rt.md:15-16 +``` +The flang driver requires two libraries for linking into any user executable, +`Fortran_main.a` and Flang-rt. +## Motivation ---------------- This statement is not quite true - the driver can do a lot without requiring these libraries. I suspect that you wanted o say that generating executables requires these libraries. [nit] flang dirver --> Flang driver ================ Comment at: flang/docs/Flang-rt.md:17 +`Fortran_main.a` and Flang-rt. +## Motivation +Before Flang-rt, the driver would need to link Fortran_main, FortranRuntime, ---------------- Motivation for ...? It's not clear what the following section aims to achieve. I think that it would be good to take a step beck and decide who this documented is intended for. If it's the end-user then, imho, this section doesn't belong here. In documentation like this I'd focus on "what" and "how" as opposed to "why". Even if this document is intended for developers, I wouldn't focus too much on the original motivations for this change and instead explain the design and its benefits: * possibility to build the runtime independently of Flang (what's the benefit of that?) * CMake set-up that's consistent with other sub-projects in LLVM, * a convenient wrapper for multiple (i.e. 2) Flang runtime libraries, * automated logic to build shared and static version of the runtime libs (to enable supporting e.g. `-static`?). I hope that this makes sense. ================ Comment at: flang/docs/Flang-rt.md:30-31 +## Fortran_main +Fortran_main is left out of Flang-rt because it is required to be linked +statically. The link type of Flang-rt can be configured with a CMake option. + ---------------- Could you document "why"? ================ Comment at: flang/docs/Flang-rt.md:35 +In order to decouple the common dependency between compiler and runtime, +FortranDecimal's sources get built a second time into a library target named +FortranDecimalRT. FortranDecimal is built for the compiler and FortranDecimalRT ---------------- Perhaps turn this into a link to the CMake definition? ================ Comment at: flang/docs/Flang-rt.md:61-63 +The two build paths mentioned above get implicitly added as library paths at the +invocation of the driver. If Flang-rt is a shared library, you must add its path +to the environment variable `LD_LIBRARY_PATH`. ---------------- 1. Please add an example. 2. "you must add its path to the environment variable `LD_LIBRARY_PATH`" <-- this is just one way to provide a search path for the dynamic linker. There are other ways too. It would be good to clarify that a) one has to make the dynamic linker aware _where_ to look and that b) `LD_LIBRARY_PATH` is one such mechanism that can be used. But I would be careful not to "endorse" it - mishandling `LD_LIBRARY_PATH` can lead to tricky surprises. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154869/new/ https://reviews.llvm.org/D154869 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
