-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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][runtime] Add dependency to build FortranRuntime after flang-new #99737
Conversation
@llvm/pr-subscribers-flang-runtime Author: Michael Klemm (mjklemm) ChangesMakefile-based builds did not have proper dependencies to built the FortranRuntime target after Flang new is available. This PR introduces a dependency to ensure that this is the case. Relates to PR #95388. Full diff: https://github.com/llvm/llvm-project/pull/99737.diff 1 Files Affected:
diff --git a/flang/runtime/CMakeLists.txt b/flang/runtime/CMakeLists.txt
index 8588af7e16eec..6d435f00b8965 100644
--- a/flang/runtime/CMakeLists.txt
+++ b/flang/runtime/CMakeLists.txt
@@ -295,3 +295,9 @@ else()
FortranRuntime.static_dbg FortranRuntime.dynamic_dbg)
endif()
set_target_properties(FortranRuntime PROPERTIES FOLDER "Flang/Runtime Libraries")
+
+# Add dependency to make sure that Fortran runtime library is being built after
+# we have the Flang compiler available. This also includes the MODULE files
+# that compile when the 'flang-new' target is built.
+add_dependencies(FortranRuntime flang-new)
+
|
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.
Thank you, Michael.
Please note that FortranRuntime
is expected to be buildable as a standalone project (see at the top of this file) - this is not going to work, and you will probably see some buildbots failures soon (e.g. https://lab.llvm.org/buildbot/#/builders/152).
It seems the module file has to be made a part of FortranRuntime
as @sscalpone suggested in the original PR.
flang/runtime/CMakeLists.txt
Outdated
# Add dependency to make sure that Fortran runtime library is being built after | ||
# we have the Flang compiler available. This also includes the MODULE files | ||
# that compile when the 'flang-new' target is built. | ||
add_dependencies(FortranRuntime flang-new module_files) |
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.
It seems just module_files
should be enough.
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.
Yeah, then we will have to rely on transitive dependencies FortranRuntime->module_files->flang-new. I'd like to be more explicit here to make sure that it's easier to understand.
If we revert the ISO_FORTRAN_ENV PR, then we will have to accept that several applications will break. Reworking the runtime build system to be a "real runtime library build", seems to be too extensive for 19.x and I had suggested to start this work for the 20.x development cycle. |
I'd appreciate hints on how to remove this dependency (for now) to re-enable runtime builds, albeit then w/o having the symbols coming from iso_fortran_env_impl.o. |
Do you have the ability to do a quick build with FortranRuntime as a standalone build with PR patched in? I think it should work, but I do not have a such a build set up to test myself. |
Hi Michael, I think it should "work" for the standalone build, except that the library will not have the module linked in. This is an acceptable workaround for the device build of BTW, my guess about failing the flang-runtime-cuda-gcc build was wrong. I think it did not fail just because |
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.
LGTM
Co-authored-by: Michael Kruse <github@meinersbur.de>
@vzakhari I interpreted standalone as FLANG_STANDALONE_BUILD=ON", but now I think you mean using the flang/runtime/CMakeLists.txt as top-level files. That indeed yields an incomplete libFortranRuntime.a because This patch doesn't change anything and since it checks the existence of the targets beforehand so will not introduce new failures. |
…ew (llvm#99737) Makefile-based builds did not have proper dependencies to built the FortranRuntime target after Flang new is available. This PR introduces a dependency to ensure that this is the case. Relates to PR llvm#95388. --------- Co-authored-by: Michael Kruse <github@meinersbur.de>
Makefile-based builds did not have proper dependencies to built the FortranRuntime target after Flang new is available. This PR introduces a dependency to ensure that this is the case. Relates to PR #95388.