[flang-rt][CMake] Avoid 'use, intrinsic ::' - #205634
Conversation
e213418 to
bb67b75
Compare
| PROPERTIES | ||
| Fortran_BUILDING_INTRINSIC_MODULES ON | ||
| Fortran_BUILDING_INSTRINSIC_MODULES ON | ||
| ) |
There was a problem hiding this comment.
Given the other changes, is this needed?
There was a problem hiding this comment.
No, not strictly. However, I thought we may keep it because it is the behaviour we actually want from CMake, just not all versions implement it yet.
nikic
left a comment
There was a problem hiding this comment.
This looks very reasonable to me, but I don't know enough about Fortran to approve it...
|
@david-arm could it be addressing the problem you're facing recently? |
|
This patch indeed fixed the |
Sorry, I just forgot about it. |
DavidTruby
left a comment
There was a problem hiding this comment.
This looks fine to me. I can't immediately see any issues with not explicitly declaring these intrinsic since there's nothing on the module path except intrinsics anyway. Users of these modules should still be able to declare their imports intrinsic if they need to.
|
I still hit an issue when doing a build. It is a scratch build. It fails in the first time. If I do |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/23925 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/29/builds/23999 Here is the relevant piece of the build log for the reference |
|
Unfortunately I'm still seeing |
Problably another implicit dependency that CMake does not see. Let me see which files don't transitivelty include it... |
|
The intermittent failure also shows up in https://lab.llvm.org/buildbot/#/builders/157/builds/47911. |
|
Please have a look at #206493. |
The PPC-specific builtin moduls have implicit dependencies insert by flang. For CMake to correctly setup the build order, they must be made explicit. This replicates the implicit module uses from https://github.com/llvm/llvm-project/blob/12b20b36737094b1c521adff0c8f58aaaf2f41fb/flang/lib/Semantics/semantics.cpp#L680-L693 in the modules themselves. The PPC-specific modules compiled by Flang-RT are listed here: https://github.com/llvm/llvm-project/blob/12b20b36737094b1c521adff0c8f58aaaf2f41fb/flang-rt/lib/runtime/CMakeLists.txt#L213-L219 This should fix the intermittend build error mentioned in #205634 (comment). Unfortunately I do not have a PowerPC (cross-)compilation build environment ready to test, nor does the Premerge-CI build it.
…493) The PPC-specific builtin moduls have implicit dependencies insert by flang. For CMake to correctly setup the build order, they must be made explicit. This replicates the implicit module uses from https://github.com/llvm/llvm-project/blob/12b20b36737094b1c521adff0c8f58aaaf2f41fb/flang/lib/Semantics/semantics.cpp#L680-L693 in the modules themselves. The PPC-specific modules compiled by Flang-RT are listed here: https://github.com/llvm/llvm-project/blob/12b20b36737094b1c521adff0c8f58aaaf2f41fb/flang-rt/lib/runtime/CMakeLists.txt#L213-L219 This should fix the intermittend build error mentioned in llvm/llvm-project#205634 (comment). Unfortunately I do not have a PowerPC (cross-)compilation build environment ready to test, nor does the Premerge-CI build it.
…493) The PPC-specific builtin moduls have implicit dependencies insert by flang. For CMake to correctly setup the build order, they must be made explicit. This replicates the implicit module uses from https://github.com/llvm/llvm-project/blob/12b20b36737094b1c521adff0c8f58aaaf2f41fb/flang/lib/Semantics/semantics.cpp#L680-L693 in the modules themselves. The PPC-specific modules compiled by Flang-RT are listed here: https://github.com/llvm/llvm-project/blob/12b20b36737094b1c521adff0c8f58aaaf2f41fb/flang-rt/lib/runtime/CMakeLists.txt#L213-L219 This should fix the intermittend build error mentioned in llvm/llvm-project#205634 (comment). Unfortunately I do not have a PowerPC (cross-)compilation build environment ready to test, nor does the Premerge-CI build it.
The PPC-specific builtin moduls have implicit dependencies insert by flang. For CMake to correctly setup the build order, they must be made explicit. This replicates the implicit module uses from https://github.com/llvm/llvm-project/blob/12b20b36737094b1c521adff0c8f58aaaf2f41fb/flang/lib/Semantics/semantics.cpp#L680-L693 in the modules themselves. The PPC-specific modules compiled by Flang-RT are listed here: https://github.com/llvm/llvm-project/blob/12b20b36737094b1c521adff0c8f58aaaf2f41fb/flang-rt/lib/runtime/CMakeLists.txt#L213-L219 This should fix the intermittend build error mentioned in llvm#205634 (comment). Unfortunately I do not have a PowerPC (cross-)compilation build environment ready to test, nor does the Premerge-CI build it.
Two build failures reported after llvm#204260 * Unix Makefiles generator stops working: The cause is that the rules for building each OBJECT library lands in its own Makefile, e.g. `flang_rt.mod.fortran.builtins.dir/build.make` and `libomp-mod.dir/build.make`. Trying to inject dependencies directly for build rules in the other file does not work. * `__ppc_types.f90` not tracked: Forgotten in llvm#204260 due to being only conditionally enabled for PowerPC targets. The solution for both is to just remove the workaround for CMake not recognizing modules uses declared using `intrinsic` which caused these problems. This PR promotes the `use` constructs in the module sources to normal dependencies that are not ignored by CMake. The `intrinsic` modifier changes the search path to only look for such a module in `-fintrinsic-modules-path`. The difference is only theoretical: 1. When compiling the intrinsic modules, there are no other search paths than for the intrinsics anyway 2. Users should strongly avoid naming their modules the same as an intrinsic module 3. Most of our intrinsic sources don't even use the `intrinsic` modifier in the first place (e.g. https://github.com/llvm/llvm-project/blob/1c9412432ff9f897d5fb096d31b945e50fbbb61d/flang-rt/lib/runtime/__cuda_builtins.f90#L13). That is, if there was a problem with lookup ambiguity, these would have already been a problem for those. 4. The checksum of used modules is only added into the .mod file (the only reason why modules would need to be transitively rebuilt) when a `use` statement appears. Many intrinsic modules don't explicitly use the modules that are added implicitly by the compiler, such as `__fortran_builtins`, and hence do not even need to be rebuilt. I started this PR with a tested-working fix ([71c5e7f](https://github.com/llvm/llvm-project/pull/205634/changes/BASE..71c5e7f0ffcf2ebd58812d2e2812b22663be1631)) to the workaround code. But given the points above, there seems to be no benefit of keeping the workaround code handling CMake version < 4.5 around just for some intrinsic modules to keep the `intrinsic` modifier with no practical effect. A non-workaround path using CMake's `Fortran_BUILDING_IN(S)TRINSIC_MODULES` setting had to be added in any case for Unix Makefile generators (reported issue 1.) because injecting dependencies is not possible with Unix Makefiles as stated, but `Fortran_BUILDING_IN(S)TRINSIC_MODULES` also does not work with the Ninja generator. Avoiding `use, intrinsic` keeps just one configuration path. I tested that modules are properly updated with Ninja and Unix Makefiles: * Build from scratch * Modifying `__fortran_builtins.f90` rebuilds its dependent modules * Modifying `iso_c_binding.f90` rebuilds its dependent modules (omp_lib.mod)
The PPC-specific builtin moduls have implicit dependencies insert by flang. For CMake to correctly setup the build order, they must be made explicit. This replicates the implicit module uses from https://github.com/llvm/llvm-project/blob/12b20b36737094b1c521adff0c8f58aaaf2f41fb/flang/lib/Semantics/semantics.cpp#L680-L693 in the modules themselves. The PPC-specific modules compiled by Flang-RT are listed here: https://github.com/llvm/llvm-project/blob/12b20b36737094b1c521adff0c8f58aaaf2f41fb/flang-rt/lib/runtime/CMakeLists.txt#L213-L219 This should fix the intermittend build error mentioned in llvm#205634 (comment). Unfortunately I do not have a PowerPC (cross-)compilation build environment ready to test, nor does the Premerge-CI build it.
Two build failures reported after #204260
Unix Makefiles generator stops working: The cause is that the rules for building each OBJECT library lands in its own Makefile, e.g.
flang_rt.mod.fortran.builtins.dir/build.makeandlibomp-mod.dir/build.make. Trying to inject dependencies directly for build rules in the other file does not work.__ppc_types.f90not tracked: Forgotten in [flang-rt][openmp] Add file-level dependencies for builtin mod files #204260 due to being only conditionally enabled for PowerPC targets.The solution for both is to just remove the workaround for CMake not recognizing modules uses declared using
intrinsicwhich caused these problems. This PR promotes theuseconstructs in the module sources to normal dependencies that are not ignored by CMake.The
intrinsicmodifier changes the search path to only look for such a module in-fintrinsic-modules-path. The difference is only theoretical:intrinsicmodifier in the first place (e.g.llvm-project/flang-rt/lib/runtime/__cuda_builtins.f90
Line 13 in 1c94124
usestatement appears. Many intrinsic modules don't explicitly use the modules that are added implicitly by the compiler, such as__fortran_builtins, and hence do not even need to be rebuilt.I started this PR with a tested-working fix (71c5e7f0ffcf2ebd58812d2e2812b22663be1631) to the workaround code. But given the points above, there seems to be no benefit of keeping the workaround code handling CMake version < 4.5 around just for some intrinsic modules to keep the
intrinsicmodifier with no practical effect. A non-workaround path using CMake'sFortran_BUILDING_IN(S)TRINSIC_MODULESsetting had to be added in any case for Unix Makefile generators (reported issue 1.) because injecting dependencies is not possible with Unix Makefiles as stated, butFortran_BUILDING_IN(S)TRINSIC_MODULESalso does not work with the Ninja generator. Avoidinguse, intrinsickeeps just one configuration path.I tested that modules are properly updated with Ninja and Unix Makefiles:
__fortran_builtins.f90rebuilds its dependent modulesiso_c_binding.f90rebuilds its dependent modules (omp_lib.mod)