-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[Flang] Remove FLANG_INCLUDE_RUNTIME #124126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Flang] Remove FLANG_INCLUDE_RUNTIME #124126
Conversation
3818238
to
b05c9a0
Compare
bd152c5
to
c515d13
Compare
Hi @Meinersbur is this the only patch that is left to be merged? Could you please rebase it so that I can try it on top-of-tree? |
8757bf4
to
698bcd0
Compare
698bcd0
to
b14ff22
Compare
Rebased and added implicitly adding |
Add `depends_on_projects=['flang-rt']`, and `checks=['check-flang-rt']` to the ppc64-flang-aix builder. The prepares the removal of the "projects" build of the flang runtime in llvm/llvm-project#124126. Split off from #333 Affected builders: * ppc64-flang-aix Affected workers: * ppc64-flang-aix-test (production) Admins listed for those workers: * LLVM on Power <[email protected]> * Mark Danial <[email protected]>
Add `enable_runtimes=['flang-rt']`, `depends_on_projects=['flang-rt']`, and `add_lit_checks=['check-flang-rt']` to all OpenMPBuilder-based builders that build flang. This prepares the removal of the "projects" build of the flang runtime in llvm/llvm-project#124126. Split off from #333 Affected builders: * openmp-offload-amdgpu-clang-flang * openmp-offload-sles-build-only * openmp-offload-rhel-9_4 * openmp-offload-rhel-8_8 Affected workers: * rocm-worker-hw-01 (staging) * rocm-worker-hw-04-sles * rocm-worker-hw-04-rhel-9_4 (staging) * rocm-worker-hw-04-rhel-8_8 (staging) Admins listed for those workers: * AMD <[email protected]>
Thank you for the rebase, @Meinersbur. Please give me some time to adjust build tools on my side to make it work with |
Add `depends_on_projects=['flang-rt']` and `checks=['check-flang-rt']` to Linaro's builders that are based on UnifiedTreeBuilder. This prepares the removal of the "projects" build of the flang runtime in llvm/llvm-project#124126. Affected builders: * flang-aarch64-dylib * flang-aarch64-sharedlibs * flang-aarch64-debug-reverse-iteration * flang-aarch64-libcxx * flang-aarch64-release * flang-aarch64-rel-assert * flang-aarch64-latest-gcc Affected workers: * linaro-flang-aarch64-dylib * linaro-flang-aarch64-sharedlibs * linaro-flang-aarch64-debug-reverse-iteration * linaro-flang-aarch64-libcxx * linaro-flang-aarch64-release * linaro-flang-aarch64-rel-assert * linaro-flang-aarch64-latest-gcc
With the option `checkout_flang=True`, ClangBuilder-based builders also compile Flang. Modify ClangBuilder to use `LLVM_ENABLE_RUNTIMES=flang-rt`. This prepares the removal of the "projects" build of the flang runtime in llvm/llvm-project#124126. Affected builders: * clang-aarch64-full-2stage * clang-aarch64-sve-vla * clang-aarch64-sve-vla-2stage * clang-aarch64-sve-vls * clang-aarch64-sve-vls-2stage * clang-aarch64-sve2-vla * clang-aarch64-sve2-vla-2stage * clang-arm64-windows-msvc * clang-arm64-windows-msvc-2stage Affected workers: * linaro-clang-aarch64-full-2stage * linaro-g3-01 * linaro-g3-02 * linaro-g3-03 * linaro-g3-04 * linaro-g4-01 * linaro-g4-02 * linaro-armv8-windows-msvc-04 * linaro-armv8-windows-msvc-02
Add `depends_on_projects=['flang-rt']`, to the amdgpu-offload-* builders. The prepares the removal of the "projects" build of the flang runtime in llvm/llvm-project#124126. The corresponding change in the LLVM repository is llvm/llvm-project#129692 Affected builders (production): * [amdgpu-offload-ubuntu-22-cmake-build-only](https://lab.llvm.org/buildbot/#/builders/203) * [amdgpu-offload-rhel-9-cmake-build-only](https://lab.llvm.org/buildbot/#/builders/205) * [amdgpu-offload-rhel-8-cmake-build-only](https://lab.llvm.org/buildbot/#/builders/204) Affected workers (production): * [rocm-docker-ubu-22](https://lab.llvm.org/buildbot/#/workers/162) * [rocm-docker-rhel-9](https://lab.llvm.org/buildbot/#/workers/163) * [rocm-docker-rhel-8](https://lab.llvm.org/buildbot/#/workers/164)
Add `depends_on_projects=['flang-rt']`, and `checks=['check-flang-rt']` to the ppc64le-flang-rhel-clang builder. The prepares the removal of the "projects" build of the flang runtime in llvm/llvm-project#124126. Affected builders (production): * [ppc64le-flang-rhel-clang](https://lab.llvm.org/buildbot/#/builders/157) Affected workers (production): * [ppc64le-flang-rhel-test](https://lab.llvm.org/buildbot/#/workers/152)
Update premerge-monolithic-windows and premerge-monolithic-linux to prepare for the removal of the "projects" build of the flang runtime in llvm/llvm-project#124126. This does not change the actual premerge GitHub action, which is done by llvm/llvm-project#128678 For premerge-monolithic-linux, add flang-rt to the LLVM_ENABLE_RUNTIMES list, indirectly by adding it to `depends_on_projects` which also updated the build scheduler. For premerge-monolithic-windows, remove building flang to match the actual pre-merge build which disabled building flang on Windows in llvm/llvm-project@e4b424a. Adding flang-rt would also require compiler-rt (which was always required on Windows, but now there is a regression test for it) and check-compiler-rt is currently failing. Split off from #333. Verified to work locally using instruction from https://llvm.org/docs/HowToAddABuilder.html#testing-a-builder-config-locally. With the exception of `-gmlt` which seems to be a Google-only extention of Clang. Affected builders: * [premerge-monolithic-windows](https://lab.llvm.org/buildbot/#/builders/35) * [premerge-monolithic-linux](https://lab.llvm.org/buildbot/#/builders/153) Affected workers: * [premerge-windows-1](https://lab.llvm.org/buildbot/#/workers/153) * [premerge-linux-1](https://lab.llvm.org/buildbot/#/workers/110)
Add `depends_on_projects=['flang-rt']` to the flang-runtime-cuda-gcc and flang-runtime-cuda-clang builders. This prepares the removal of the "projects" build of the flang runtime in llvm/llvm-project#124126. Affected builders: * flang-runtime-cuda-gcc This previously only built the runtime using the top-level CMakeLists.txt in `flang/runtime/CMakeLists.txt`. This is going to be replaced with the "standalone runtimes build", with the top-level `runtimes/CMakeLists.txt`. This still needs Flang to successed, hence replacing with a bootstrap-build where the `FLANG_RT_*` options are internally forwarded to the runtimes build. * flang-runtime-cuda-clang This is a manual bootstrapping build which first compiles Clang, then the runtime out-of-tree. This is replaced with a standalone runtimes build as described above. Because it needs Flang, also adding Flang to the enabled projects of the stage1 build. Neither build runs the `check-*` targets, probably due to the lack of actual CUDA hardware which running the runtime unittests require. Affected workers: * as-builder-7
Make `FlangBuilder.getFlangOutOfTreeBuildFactory` build the flang-rt runtime as an out-of-tree runtimes build with additional steps. Fixes some issues with FlangBuilder too: * Clears the build directories with the clean_obj flag set. Usually buildbot deletes the build directory whenever a CMakeLists.txt in a depends_on_projects subdirectory changes to get a clean build. Otherwise, an incompatible CMakeCache.txt will carry on forever. * Add `flang` and `flang-rt` as depends_on_projects. Otherwise, changes in these subdirectories do not trigger a build. Even worse, breaking commits will be attributed to the next commit in llvm/clang/mlir/openmp, as happed in e.g. https://lab.llvm.org/buildbot/#/builders/53/builds/11759 (commit causing the failure was llvm/llvm-project@fe8b323 ) This PR is not strictly necessary for llvm/llvm-project#124126, it just wouldn't build the runtime anymore. Affected builders: * flang-aarch64-out-of-tree Affected workers: * linaro-flang-aarch64-out-of-tree Tested locally on x86_64.
llvm/CMakeLists.txt
Outdated
message(STATUS "Enabling Flang-RT as a dependency of Flang" | ||
"Flang-RT is required for a fully working Flang toolchain." | ||
"If you intend to compile Flang-RT separately following the instructions at" | ||
"https://github.com/llvm/llvm-project/blob/main/flang-rt/README.md#standalone-runtimes-build" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Directly linking to a github readme is a little weird. Do we not have a documentation page for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no flang-rt.llvm.org . I do find several links to github.com in llvm/docs, such as
llvm-project/llvm/docs/TableGen/index.rst
Line 291 in 9387281
`README <https://github.com/llvm/llvm-project/blob/main/llvm/utils/TableGen/README.md>`_. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's https://flang.llvm.org/docs/ right? it's not a blocking issue, I just feel like build configs are better shared through CMake config files or actual documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it ok to update the docs in a follow-up? Consolidating this README.md and https://flang.llvm.org/docs/GettingStarted.html#building-flang itself would be a larger patch. Also think that advanced topics like FLANG_RT_EXPERIMENTAL_OFFLOAD_SUPPORT does not belong into a GettingStarted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved the help message(...)
to flang/ so a message is also emitted in a flang-standalone build (which is does not process LLVM_ENABLE_RUNTIMES at all). I also shortened it without the README link, may update it after consolidation.
@@ -1,9 +0,0 @@ | |||
# This test is not run by default as it requires input. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guessing this was already copied somewhere else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
flang-rt version of this example is here: https://github.com/llvm/llvm-project/blob/main/flang-rt/examples/ExternalHelloWorld/CMakeLists.txt
This is an example that links to flang-rt from C++. No use of a Fortran compiler. The .cpp itself was moved in #110298
Everything is ready on my side. Thank you for waiting, Michael! LGTM |
Flang's runtime can now be built using LLVM's LLVM_ENABLE_RUNTIMES mechanism, with the intent to remove the old mechanism in llvm#124126. Update the pre-merge builders to use the new mechanism. In the current form, llvm#124126 actually will add LLVM_ENABLE_RUNTIMES=flang-rt implicitly, so no change is strictly needed. I still think it is a good idea to do it explicitly and in advance. On Windows, flang-rt also requires compiler-rt, but which is not building on Windows anyway.
After AMD gave their OK as well, I am going to land this. |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/152/builds/1292 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/7/builds/12751 Here is the relevant piece of the build log for the reference
|
The production buildbot master apparently has not yet been restarted since llvm/llvm-zorg#393 landed. This reverts commit 96d1bae.
The two builders failing were supposed to be updated by llvm/llvm-zorg#393. Have to wait until https://lab.llvm.org/buildbot/ is restarted before relanding. |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/169/builds/9845 Here is the relevant piece of the build log for the reference
|
This reverts commit 27539c3. Retry with new buildbot configuration after master restart. Original message: Remove the FLANG_INCLUDE_RUNTIME option which was replaced by LLVM_ENABLE_RUNTIMES=flang-rt. The FLANG_INCLUDE_RUNTIME option was added in #122336 which disables the non-runtimes build instructions for the Flang runtime so they do not conflict with the LLVM_ENABLE_RUNTIMES=flang-rt option added in #110217. In order to not maintain multiple build instructions for the same thing, this PR completely removes the old build instructions (effectively forcing FLANG_INCLUDE_RUNTIME=OFF). As per discussion in https://discourse.llvm.org/t/buildbot-changes-with-llvm-enable-runtimes-flang-rt/83571/2 we now implicitly add LLVM_ENABLE_RUNTIMES=flang-rt whenever Flang is compiled in a bootstrapping (non-standalone) build. Because it is possible to build Flang-RT separately, this behavior can be disabled using `-DFLANG_ENABLE_FLANG_RT=OFF`. Also see the discussion an implicitly adding runtimes/projects in #123964.
This reverts commit 77581e2.
This patch seems to have removed the behaviour that flang-rt is added to LLVM_ENABLE_RUNTIMES automatically which has broken all of our internal builds. My read of the commit message, and the patch, is that this behaviour hasn't been intentionally removed?
and I am not getting flang-rt. I also see:
|
option(FLANG_ENABLE_FLANG_RT "Implicitly add LLVM_ENABLE_RUNTIMES=flang-rt when compiling Flang" ON) | ||
if (FLANG_ENABLE_FLANG_RT AND NOT "flang-rt" IN_LIST LLVM_ENABLE_RUNTIMES) | ||
message(STATUS "Enabling Flang-RT as a dependency of Flang") | ||
list(APPEND LLVM_ENABLE_RUNTIMES "flang-rt") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe you can't list(APPEND ..)
cache variables like this as they'll get overridden by what was provided on the command line. I think it should be set(LLVM_ENABLE_RUNTIMES "${LLVM_ENABLE_RUNTIMES};flang-rt" CACHE STRING "")
or something along those lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, that doesn't seem to work either. I actually don't see the message so for some reason we aren't entering that if statement body I guess?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry. All of that was false alarms. The issue is that LLVM_ENABLE_RUNTIMES hasn't been defined by this point, it's defined a little after these checks. When it does get defined it overwrites whatever was done here.
If you move the LLVM_ENABLE_RUNTIMES define above this, then this works as expected.
I have posted #138136 which fixes the issues I mentioned above |
This reverts commit 27539c3. Retry with new buildbot configuration after master restart. Original message: Remove the FLANG_INCLUDE_RUNTIME option which was replaced by LLVM_ENABLE_RUNTIMES=flang-rt. The FLANG_INCLUDE_RUNTIME option was added in llvm#122336 which disables the non-runtimes build instructions for the Flang runtime so they do not conflict with the LLVM_ENABLE_RUNTIMES=flang-rt option added in llvm#110217. In order to not maintain multiple build instructions for the same thing, this PR completely removes the old build instructions (effectively forcing FLANG_INCLUDE_RUNTIME=OFF). As per discussion in https://discourse.llvm.org/t/buildbot-changes-with-llvm-enable-runtimes-flang-rt/83571/2 we now implicitly add LLVM_ENABLE_RUNTIMES=flang-rt whenever Flang is compiled in a bootstrapping (non-standalone) build. Because it is possible to build Flang-RT separately, this behavior can be disabled using `-DFLANG_ENABLE_FLANG_RT=OFF`. Also see the discussion an implicitly adding runtimes/projects in llvm#123964.
This reverts commit 27539c3. Retry with new buildbot configuration after master restart. Original message: Remove the FLANG_INCLUDE_RUNTIME option which was replaced by LLVM_ENABLE_RUNTIMES=flang-rt. The FLANG_INCLUDE_RUNTIME option was added in llvm#122336 which disables the non-runtimes build instructions for the Flang runtime so they do not conflict with the LLVM_ENABLE_RUNTIMES=flang-rt option added in llvm#110217. In order to not maintain multiple build instructions for the same thing, this PR completely removes the old build instructions (effectively forcing FLANG_INCLUDE_RUNTIME=OFF). As per discussion in https://discourse.llvm.org/t/buildbot-changes-with-llvm-enable-runtimes-flang-rt/83571/2 we now implicitly add LLVM_ENABLE_RUNTIMES=flang-rt whenever Flang is compiled in a bootstrapping (non-standalone) build. Because it is possible to build Flang-RT separately, this behavior can be disabled using `-DFLANG_ENABLE_FLANG_RT=OFF`. Also see the discussion an implicitly adding runtimes/projects in llvm#123964.
This reverts commit 27539c3. Retry with new buildbot configuration after master restart. Original message: Remove the FLANG_INCLUDE_RUNTIME option which was replaced by LLVM_ENABLE_RUNTIMES=flang-rt. The FLANG_INCLUDE_RUNTIME option was added in llvm#122336 which disables the non-runtimes build instructions for the Flang runtime so they do not conflict with the LLVM_ENABLE_RUNTIMES=flang-rt option added in llvm#110217. In order to not maintain multiple build instructions for the same thing, this PR completely removes the old build instructions (effectively forcing FLANG_INCLUDE_RUNTIME=OFF). As per discussion in https://discourse.llvm.org/t/buildbot-changes-with-llvm-enable-runtimes-flang-rt/83571/2 we now implicitly add LLVM_ENABLE_RUNTIMES=flang-rt whenever Flang is compiled in a bootstrapping (non-standalone) build. Because it is possible to build Flang-RT separately, this behavior can be disabled using `-DFLANG_ENABLE_FLANG_RT=OFF`. Also see the discussion an implicitly adding runtimes/projects in llvm#123964.
This reverts commit 27539c3. Retry with new buildbot configuration after master restart. Original message: Remove the FLANG_INCLUDE_RUNTIME option which was replaced by LLVM_ENABLE_RUNTIMES=flang-rt. The FLANG_INCLUDE_RUNTIME option was added in llvm#122336 which disables the non-runtimes build instructions for the Flang runtime so they do not conflict with the LLVM_ENABLE_RUNTIMES=flang-rt option added in llvm#110217. In order to not maintain multiple build instructions for the same thing, this PR completely removes the old build instructions (effectively forcing FLANG_INCLUDE_RUNTIME=OFF). As per discussion in https://discourse.llvm.org/t/buildbot-changes-with-llvm-enable-runtimes-flang-rt/83571/2 we now implicitly add LLVM_ENABLE_RUNTIMES=flang-rt whenever Flang is compiled in a bootstrapping (non-standalone) build. Because it is possible to build Flang-RT separately, this behavior can be disabled using `-DFLANG_ENABLE_FLANG_RT=OFF`. Also see the discussion an implicitly adding runtimes/projects in llvm#123964.
This reverts commit 27539c3. Retry with new buildbot configuration after master restart. Original message: Remove the FLANG_INCLUDE_RUNTIME option which was replaced by LLVM_ENABLE_RUNTIMES=flang-rt. The FLANG_INCLUDE_RUNTIME option was added in llvm#122336 which disables the non-runtimes build instructions for the Flang runtime so they do not conflict with the LLVM_ENABLE_RUNTIMES=flang-rt option added in llvm#110217. In order to not maintain multiple build instructions for the same thing, this PR completely removes the old build instructions (effectively forcing FLANG_INCLUDE_RUNTIME=OFF). As per discussion in https://discourse.llvm.org/t/buildbot-changes-with-llvm-enable-runtimes-flang-rt/83571/2 we now implicitly add LLVM_ENABLE_RUNTIMES=flang-rt whenever Flang is compiled in a bootstrapping (non-standalone) build. Because it is possible to build Flang-RT separately, this behavior can be disabled using `-DFLANG_ENABLE_FLANG_RT=OFF`. Also see the discussion an implicitly adding runtimes/projects in llvm#123964.
Remove the FLANG_INCLUDE_RUNTIME option which was replaced by LLVM_ENABLE_RUNTIMES=flang-rt.
The FLANG_INCLUDE_RUNTIME option was added in #122336 which disables the non-runtimes build instructions for the Flang runtime so they do not conflict with the LLVM_ENABLE_RUNTIMES=flang-rt option added in #110217. In order to not maintain multiple build instructions for the same thing, this PR completely removes the old build instructions (effectively forcing FLANG_INCLUDE_RUNTIME=OFF).
As per discussion in https://discourse.llvm.org/t/buildbot-changes-with-llvm-enable-runtimes-flang-rt/83571/2 we now implicitly add LLVM_ENABLE_RUNTIMES=flang-rt whenever Flang is compiled in a bootstrapping (non-standalone) build. Because it is possible to build Flang-RT separately, this behavior can be disabled using
-DFLANG_ENABLE_FLANG_RT=OFF
. Also see the discussion an implicitly adding runtimes/projects in #123964.This PR will remains in draft status until the buildbots' configurations have been updated. Due to LLVM_ENABLE_RUNTIMES=flang-rt being added automatically, most of them would probably continue to work, except the out-of-tree builders which require some adjustment.
Series of buildbot updates: