-
Notifications
You must be signed in to change notification settings - Fork 12.3k
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] Build ISO_FORTRAN_ENV to export kind arrays as linkable symbols #95388
Conversation
@llvm/pr-subscribers-flang-runtime @llvm/pr-subscribers-flang-driver Author: Michael Klemm (mjklemm) ChangesMoves definitions of the kind arrays into a Fortran MODULE to not only emit the MOD file, but also compile that MODULE file into an object file. This file is then linked into libFortranRuntime.so. Eventually this workaround PR shoud be redone and a proper runtime build should be setup that will then also compile Fortran MODULE files. Fixes #89403 Full diff: https://github.com/llvm/llvm-project/pull/95388.diff 6 Files Affected:
diff --git a/flang/CMakeLists.txt b/flang/CMakeLists.txt
index cbe8f1186236a..070c39eb6e9ab 100644
--- a/flang/CMakeLists.txt
+++ b/flang/CMakeLists.txt
@@ -538,3 +538,4 @@ get_clang_resource_dir(HEADER_INSTALL_DIR SUBDIR include)
install(
FILES include/flang/ISO_Fortran_binding.h
DESTINATION ${HEADER_INSTALL_DIR} )
+
diff --git a/flang/module/__fortran_builtin_kinds.f90 b/flang/module/__fortran_builtin_kinds.f90
new file mode 100644
index 0000000000000..189055dc26c0c
--- /dev/null
+++ b/flang/module/__fortran_builtin_kinds.f90
@@ -0,0 +1,110 @@
+!===-- module/__fortran_bultin_kinds.f90 --=--------------------------------===!
+!
+! Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+! See https://llvm.org/LICENSE.txt for license information.
+! SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+!
+!===------------------------------------------------------------------------===!
+
+module __fortran_builtin_kinds
+ implicit none
+ private
+
+ ! INTEGER types
+ integer, parameter, public :: &
+ selectedInt8 = selected_int_kind(2), &
+ selectedInt16 = selected_int_kind(4), &
+ selectedInt32 = selected_int_kind(9), &
+ selectedInt64 = selected_int_kind(18),&
+ selectedInt128 = selected_int_kind(38), &
+ safeInt8 = merge(selectedInt8, selected_int_kind(0), &
+ selectedInt8 >= 0), &
+ safeInt16 = merge(selectedInt16, selected_int_kind(0), &
+ selectedInt16 >= 0), &
+ safeInt32 = merge(selectedInt32, selected_int_kind(0), &
+ selectedInt32 >= 0), &
+ safeInt64 = merge(selectedInt64, selected_int_kind(0), &
+ selectedInt64 >= 0), &
+ safeInt128 = merge(selectedInt128, selected_int_kind(0), &
+ selectedInt128 >= 0)
+
+ integer, parameter, public :: &
+ int8 = merge(selectedInt8, merge(-2, -1, selectedInt8 >= 0), &
+ digits(int(0,kind=safeInt8)) == 7), &
+ int16 = merge(selectedInt16, merge(-2, -1, selectedInt16 >= 0), &
+ digits(int(0,kind=safeInt16)) == 15), &
+ int32 = merge(selectedInt32, merge(-2, -1, selectedInt32 >= 0), &
+ digits(int(0,kind=safeInt32)) == 31), &
+ int64 = merge(selectedInt64, merge(-2, -1, selectedInt64 >= 0), &
+ digits(int(0,kind=safeInt64)) == 63), &
+ int128 = merge(selectedInt128, merge(-2, -1, selectedInt128 >= 0), &
+ digits(int(0,kind=safeInt128)) == 127)
+
+ integer, parameter, dimension(*), public :: __builtin_integer_kinds = [ &
+ selected_int_kind(0), &
+ [(pack([selected_int_kind(k)], &
+ selected_int_kind(k) >= 0 .and. &
+ selected_int_kind(k) /= selected_int_kind(k-1)), &
+ integer :: k=1, 39)]]
+
+ ! LOGICAL TYPES
+ integer, parameter, public :: &
+ logical8 = int8, logical16 = int16, logical32 = int32, logical64 = int64
+
+ integer, parameter, dimension(*), public :: __builtin_logical_kinds = [ &
+ pack([logical8], logical8 >= 0), &
+ pack([logical16], logical16 >= 0), &
+ pack([logical32], logical32 >= 0), &
+ pack([logical64], logical64 >= 0) &
+ ]
+
+ ! REAL types
+ integer, parameter, public :: &
+ selectedReal16 = selected_real_kind(3, 4), & ! IEEE half
+ selectedBfloat16 = selected_real_kind(2, 37), & ! truncated IEEE single
+ selectedReal32 = selected_real_kind(6, 37), & ! IEEE single
+ selectedReal64 = selected_real_kind(15, 307), & ! IEEE double
+ selectedReal80 = selected_real_kind(18, 4931), & ! 80x87 extended
+ selectedReal64x2 = selected_real_kind(31, 307), & ! "double-double"
+ selectedReal128 = selected_real_kind(33, 4931), & ! IEEE quad
+ safeReal16 = merge(selectedReal16, selected_real_kind(0,0), &
+ selectedReal16 >= 0), &
+ safeBfloat16 = merge(selectedBfloat16, selected_real_kind(0,0), &
+ selectedBfloat16 >= 0), &
+ safeReal32 = merge(selectedReal32, selected_real_kind(0,0), &
+ selectedReal32 >= 0), &
+ safeReal64 = merge(selectedReal64, selected_real_kind(0,0), &
+ selectedReal64 >= 0), &
+ safeReal80 = merge(selectedReal80, selected_real_kind(0,0), &
+ selectedReal80 >= 0), &
+ safeReal64x2 = merge(selectedReal64x2, selected_real_kind(0,0), &
+ selectedReal64x2 >= 0), &
+ safeReal128 = merge(selectedReal128, selected_real_kind(0,0), &
+ selectedReal128 >= 0)
+
+ integer, parameter, public :: &
+ real16 = merge(selectedReal16, merge(-2, -1, selectedReal16 >= 0), &
+ digits(real(0,kind=safeReal16)) == 11), &
+ bfloat16 = merge(selectedBfloat16, merge(-2, -1, selectedBfloat16 >= 0), &
+ digits(real(0,kind=safeBfloat16)) == 8), &
+ real32 = merge(selectedReal32, merge(-2, -1, selectedReal32 >= 0), &
+ digits(real(0,kind=safeReal32)) == 24), &
+ real64 = merge(selectedReal64, merge(-2, -1, selectedReal64 >= 0), &
+ digits(real(0,kind=safeReal64)) == 53), &
+ real80 = merge(selectedReal80, merge(-2, -1, selectedReal80 >= 0), &
+ digits(real(0,kind=safeReal80)) == 64), &
+ real64x2 = merge(selectedReal64x2, merge(-2, -1, selectedReal64x2 >= 0), &
+ digits(real(0,kind=safeReal64x2)) == 106), &
+ real128 = merge(selectedReal128, merge(-2, -1, selectedReal128 >= 0), &
+ digits(real(0,kind=safeReal128)) == 113)
+
+ integer, parameter, dimension(*), public :: __builtin_real_kinds = [ &
+ pack([real16], real16 >= 0), &
+ pack([bfloat16], bfloat16 >= 0), &
+ pack([real32], real32 >= 0), &
+ pack([real64], real64 >= 0), &
+ pack([real80], real80 >= 0), &
+ pack([real64x2], real64x2 >= 0), &
+ pack([real128], real128 >= 0) &
+ ]
+end module __fortran_builtin_kinds
\ No newline at end of file
diff --git a/flang/module/iso_fortran_env.f90 b/flang/module/iso_fortran_env.f90
index 6ca98e518aeac..f751ba03f7bac 100644
--- a/flang/module/iso_fortran_env.f90
+++ b/flang/module/iso_fortran_env.f90
@@ -22,6 +22,23 @@ module iso_fortran_env
compiler_options => __builtin_compiler_options, &
compiler_version => __builtin_compiler_version
+ use __fortran_builtin_kinds, only: &
+ selectedInt8, selectedInt16, selectedInt32, selectedInt64, selectedInt128, &
+ safeInt8, safeInt16, safeInt32, safeInt64, safeInt128, &
+ int8, int16, int32, int64, int128, &
+ logical8, logical16, logical32, logical64, &
+ selectedReal16, selectedBfloat16, selectedReal32, &
+ selectedReal64, selectedReal80, selectedReal64x2, &
+ selectedReal128, &
+ safeReal16, safeBfloat16, safeReal32, &
+ safeReal64, safeReal80, safeReal64x2, &
+ safeReal128, &
+ real16, bfloat16, real32, real64, &
+ real80, real64x2, real128, &
+ integer_kinds => __builtin_integer_kinds, &
+ real_kinds => __builtin_real_kinds, &
+ logical_kinds => __builtin_logical_kinds
+
implicit none
private
@@ -38,95 +55,22 @@ module iso_fortran_env
pack([selectedUCS_2], selectedUCS_2 >= 0), &
pack([selectedUnicode], selectedUnicode >= 0)]
- integer, parameter :: &
- selectedInt8 = selected_int_kind(2), &
- selectedInt16 = selected_int_kind(4), &
- selectedInt32 = selected_int_kind(9), &
- selectedInt64 = selected_int_kind(18),&
- selectedInt128 = selected_int_kind(38), &
- safeInt8 = merge(selectedInt8, selected_int_kind(0), &
- selectedInt8 >= 0), &
- safeInt16 = merge(selectedInt16, selected_int_kind(0), &
- selectedInt16 >= 0), &
- safeInt32 = merge(selectedInt32, selected_int_kind(0), &
- selectedInt32 >= 0), &
- safeInt64 = merge(selectedInt64, selected_int_kind(0), &
- selectedInt64 >= 0), &
- safeInt128 = merge(selectedInt128, selected_int_kind(0), &
- selectedInt128 >= 0)
- integer, parameter, public :: &
- int8 = merge(selectedInt8, merge(-2, -1, selectedInt8 >= 0), &
- digits(int(0,kind=safeInt8)) == 7), &
- int16 = merge(selectedInt16, merge(-2, -1, selectedInt16 >= 0), &
- digits(int(0,kind=safeInt16)) == 15), &
- int32 = merge(selectedInt32, merge(-2, -1, selectedInt32 >= 0), &
- digits(int(0,kind=safeInt32)) == 31), &
- int64 = merge(selectedInt64, merge(-2, -1, selectedInt64 >= 0), &
- digits(int(0,kind=safeInt64)) == 63), &
- int128 = merge(selectedInt128, merge(-2, -1, selectedInt128 >= 0), &
- digits(int(0,kind=safeInt128)) == 127)
-
- integer, parameter, public :: integer_kinds(*) = [ &
- selected_int_kind(0), &
- [(pack([selected_int_kind(k)], &
- selected_int_kind(k) >= 0 .and. &
- selected_int_kind(k) /= selected_int_kind(k-1)), &
- integer :: k=1, 39)]]
+ public :: selectedInt8, selectedInt16, selectedInt32, selectedInt64, selectedInt128, &
+ safeInt8, safeInt16, safeInt32, safeInt64, safeInt128, &
+ int8, int16, int32, int64, int128
- integer, parameter, public :: &
- logical8 = int8, logical16 = int16, logical32 = int32, logical64 = int64
- integer, parameter, public :: logical_kinds(*) = [ &
- pack([logical8], logical8 >= 0), &
- pack([logical16], logical16 >= 0), &
- pack([logical32], logical32 >= 0), &
- pack([logical64], logical64 >= 0)]
+ public :: logical8, logical16, logical32, logical64
- integer, parameter :: &
- selectedReal16 = selected_real_kind(3, 4), & ! IEEE half
- selectedBfloat16 = selected_real_kind(2, 37), & ! truncated IEEE single
- selectedReal32 = selected_real_kind(6, 37), & ! IEEE single
- selectedReal64 = selected_real_kind(15, 307), & ! IEEE double
- selectedReal80 = selected_real_kind(18, 4931), & ! 80x87 extended
- selectedReal64x2 = selected_real_kind(31, 307), & ! "double-double"
- selectedReal128 = selected_real_kind(33, 4931), & ! IEEE quad
- safeReal16 = merge(selectedReal16, selected_real_kind(0,0), &
- selectedReal16 >= 0), &
- safeBfloat16 = merge(selectedBfloat16, selected_real_kind(0,0), &
- selectedBfloat16 >= 0), &
- safeReal32 = merge(selectedReal32, selected_real_kind(0,0), &
- selectedReal32 >= 0), &
- safeReal64 = merge(selectedReal64, selected_real_kind(0,0), &
- selectedReal64 >= 0), &
- safeReal80 = merge(selectedReal80, selected_real_kind(0,0), &
- selectedReal80 >= 0), &
- safeReal64x2 = merge(selectedReal64x2, selected_real_kind(0,0), &
- selectedReal64x2 >= 0), &
- safeReal128 = merge(selectedReal128, selected_real_kind(0,0), &
- selectedReal128 >= 0)
- integer, parameter, public :: &
- real16 = merge(selectedReal16, merge(-2, -1, selectedReal16 >= 0), &
- digits(real(0,kind=safeReal16)) == 11), &
- bfloat16 = merge(selectedBfloat16, merge(-2, -1, selectedBfloat16 >= 0), &
- digits(real(0,kind=safeBfloat16)) == 8), &
- real32 = merge(selectedReal32, merge(-2, -1, selectedReal32 >= 0), &
- digits(real(0,kind=safeReal32)) == 24), &
- real64 = merge(selectedReal64, merge(-2, -1, selectedReal64 >= 0), &
- digits(real(0,kind=safeReal64)) == 53), &
- real80 = merge(selectedReal80, merge(-2, -1, selectedReal80 >= 0), &
- digits(real(0,kind=safeReal80)) == 64), &
- real64x2 = merge(selectedReal64x2, merge(-2, -1, selectedReal64x2 >= 0), &
- digits(real(0,kind=safeReal64x2)) == 106), &
- real128 = merge(selectedReal128, merge(-2, -1, selectedReal128 >= 0), &
- digits(real(0,kind=safeReal128)) == 113)
-
- integer, parameter, public :: real_kinds(*) = [ &
- pack([real16], real16 >= 0), &
- pack([bfloat16], bfloat16 >= 0), &
- pack([real32], real32 >= 0), &
- pack([real64], real64 >= 0), &
- pack([real80], real80 >= 0), &
- pack([real64x2], real64x2 >= 0), &
- pack([real128], real128 >= 0)]
+ public :: selectedReal16, selectedBfloat16, selectedReal32, &
+ selectedReal64, selectedReal80, selectedReal64x2, &
+ selectedReal128, &
+ safeReal16, safeBfloat16, safeReal32, &
+ safeReal64, safeReal80, safeReal64x2, &
+ safeReal128, &
+ real16, bfloat16, real32, real64, &
+ real80, real64x2, real128
+
+ public :: integer_kinds, real_kinds, logical_kinds
integer, parameter, public :: current_team = -1, &
initial_team = -2, &
diff --git a/flang/runtime/CMakeLists.txt b/flang/runtime/CMakeLists.txt
index a826980e19411..c3bec4043e760 100644
--- a/flang/runtime/CMakeLists.txt
+++ b/flang/runtime/CMakeLists.txt
@@ -260,6 +260,7 @@ if (NOT DEFINED MSVC)
INSTALL_WITH_TOOLCHAIN
)
+ target_link_libraries(FortranRuntime PRIVATE ${FORTRAN_MODULE_OBJECTS})
else()
add_flang_library(FortranRuntime
${sources}
diff --git a/flang/tools/f18/CMakeLists.txt b/flang/tools/f18/CMakeLists.txt
index 4771199651602..5f4c49e42c36e 100644
--- a/flang/tools/f18/CMakeLists.txt
+++ b/flang/tools/f18/CMakeLists.txt
@@ -4,7 +4,16 @@ set(LLVM_LINK_COMPONENTS
Support
)
-set(MODULES
+# Define the list of Fortran module files that need to be compiled
+# to produce an object file for inclusion into the FortranRuntime
+# library.
+set(MODULES_WITH_IMPLEMENTATION
+ "__fortran_builtin_kinds"
+)
+
+# Define the list of Fortran module files for which it is
+# sufficient to generate the module file via -fsyntax-only.
+set(MODULES_WITHOUT_IMPLEMENTATION
"__fortran_builtins"
"__fortran_ieee_exceptions"
"__fortran_type_info"
@@ -20,6 +29,12 @@ set(MODULES
"iso_fortran_env"
)
+set(MODULES ${MODULES_WITH_IMPLEMENTATION} ${MODULES_WITHOUT_IMPLEMENTATION})
+
+# Init variable to hold extra object files coming from the Fortran modules;
+# these module files will be contributed from the CMakeLists in flang/tools/f18.
+unset(FORTRAN_MODULE_OBJECTS CACHE)
+
# Create module files directly from the top-level module source directory.
# If CMAKE_CROSSCOMPILING, then the newly built flang-new executable was
# cross compiled, and thus can't be executed on the build system and thus
@@ -41,6 +56,9 @@ if (NOT CMAKE_CROSSCOMPILING)
if(NOT ${filename} STREQUAL "__fortran_type_info")
set(depends ${depends} ${FLANG_INTRINSIC_MODULES_DIR}/__fortran_type_info.mod)
endif()
+ if(${filename} STREQUAL "iso_fortran_env")
+ set(depends ${depends} ${FLANG_INTRINSIC_MODULES_DIR}/__fortran_builtin_kinds.mod)
+ endif()
if(${filename} STREQUAL "ieee_arithmetic" OR
${filename} STREQUAL "ieee_exceptions")
set(depends ${depends} ${FLANG_INTRINSIC_MODULES_DIR}/__fortran_ieee_exceptions.mod)
@@ -58,16 +76,34 @@ if (NOT CMAKE_CROSSCOMPILING)
endif()
endif()
+
+ # Some modules have an implementation part that needs to be added to the
+ # FortranRuntime library.
+ set(compile_with "-fsyntax-only")
+ set(object_output "")
+ set(include_in_link FALSE)
+ if(${filename} IN_LIST MODULES_WITH_IMPLEMENTATION)
+ set(compile_with "-c")
+ set(object_output "${CMAKE_CURRENT_BINARY_DIR}/${filename}${CMAKE_CXX_OUTPUT_EXTENSION}")
+ set(include_in_link TRUE)
+ endif()
+
set(base ${FLANG_INTRINSIC_MODULES_DIR}/${filename})
# TODO: We may need to flag this with conditional, in case Flang is built w/o OpenMP support
- add_custom_command(OUTPUT ${base}.mod
+ add_custom_command(OUTPUT ${base}.mod ${object_output}
COMMAND ${CMAKE_COMMAND} -E make_directory ${FLANG_INTRINSIC_MODULES_DIR}
- COMMAND flang-new ${opts} -cpp -fsyntax-only -module-dir ${FLANG_INTRINSIC_MODULES_DIR}
+ COMMAND flang-new ${opts} -cpp ${compile_with} -module-dir ${FLANG_INTRINSIC_MODULES_DIR}
${FLANG_SOURCE_DIR}/module/${filename}.f90
DEPENDS flang-new ${FLANG_SOURCE_DIR}/module/${filename}.f90 ${FLANG_SOURCE_DIR}/module/__fortran_builtins.f90 ${depends}
)
list(APPEND MODULE_FILES ${base}.mod)
install(FILES ${base}.mod DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/flang")
+
+ # If a module has been compiled into an object file, add the file to
+ # the link line for the FortranRuntime library.
+ if(include_in_link)
+ set(FORTRAN_MODULE_OBJECTS "${FORTRAN_MODULE_OBJECTS}" "${object_output}" CACHE INTERNAL "")
+ endif()
endforeach()
# Special case for omp_lib.mod, because its source comes from openmp/runtime/src/include.
diff --git a/flang/tools/flang-driver/CMakeLists.txt b/flang/tools/flang-driver/CMakeLists.txt
index ce30ecff028dc..5bf535b18ad78 100644
--- a/flang/tools/flang-driver/CMakeLists.txt
+++ b/flang/tools/flang-driver/CMakeLists.txt
@@ -19,8 +19,6 @@ add_flang_tool(flang-new
# These libraries are used in the linker invocation generated by the driver
# (i.e. when constructing the linker job). Without them the driver would be
# unable to generate executables.
- FortranRuntime
- FortranDecimal
)
target_link_libraries(flang-new
|
@@ -0,0 +1,110 @@ | |||
!===-- module/__fortran_bultin_kinds.f90 --=--------------------------------===! |
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.
why can't these definitions remain in iso_fortran_env?
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 was giving me compiler errors for numeric_storage_size, which seems it needs to not be compiled as part of the module file but rather as part of when the module has been included into the host routine.
That's I basically went back to my original idea of how I wanted to fix the bug and moved the intrinsic definitions into their own module file.
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.
__builtin_numeric_storage_size() gets special treatment in folding that can easily be adjusted without this kind of disruption.
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.
Can you provide a hint how this could be done? I'm not that deep into the inner workings for Flang yet to see how this can be done.
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.
Grep for builtin_numeric_storage
and you'll see what's going on.
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.
In flang/lib/Evaluate/fold-integer.cpp
ca. line 1340 is the code that folds (underscore underscore) builtin_numeric_storage
. It defers that folding today until the reference to that intrinsic function is coming from a module file, at which point the size of a default numeric storage unit must be known. If you're changing things so that ISO_FORTRAN_ENV
is now going to be compiled with the size of a default numeric storage unit being well-defined, then you can probably just remove the code that defers the folding.
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.
When I remove the code, the compiler folds the current value 32_4
into the iso_fortran_env.mod
file. From there on, numeric_storage_size
always reports 32 as the storage size, regardless of compiler options.
The code removed:
if (!context.moduleFileName()) {
// Don't fold this reference until it appears in the module file
// for ISO_FORTRAN_ENV -- the value depends on the compiler options
// that might be in force.
} 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.
Yes, but since you now need to also compile the modules into distinct *.o
files for each combination of target and relevant compilation options, isn't that what has to happen?
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.
That's not needed, if we do the separation like I'm proposing in this PR. Everything that is compiled into an object file is independent of the relevant compiler options (like -fdefault-real-8
), while numeric_storage_size
is left in iso_fortran_env
that is only treated with -fsyntax-only
, which resolves the whole issue w/o touching the compiler itself.
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.
And complicated module file shenanigans are better somehow than "touching" the compiler? I guess that's a matter of opinion. I'll let you find some other reviewer for this, but you can help them out by at least documenting what's going on here with some comments, and fixing the name of the file in its header.
Did you move the definitions because you get an error compiling the full This is the part of the module I'm talking about:
|
Thanks for looking at this, @mjklemm ! My thought was to move the build of the module files to be part of the runtime build. One advantage is that the object file can be put into the libFortranRuntime. The other is that the module file build is more closely associated with the target instead of the host. |
I consider this a first step to resolve the bug, albeit with a workaround. In the long run, I totally agree that the Fortran runtime part should be built like any other LLVM runtime. As part of that the module file should be considered to move to be within in the runtime folder and also the CMake for it should move from tools/f18 into the runtime folder. I think that would greatly simplify the CMake code needed. |
Yes, exactly. I think we can leave this as is if we feel that moving the compiled part of iso_fortran_env into its own module file like I did is the right thing. Not sure if we would need this for other stuff in the future, but maybe splitting the intrinsic modules into an implementation file and a definitions file might be something to consider going forward. |
Moves definitions of the kind arrays into a Fortran MODULE to not only emit the MOD file, but also compile that MODULE file into an object file. This file is then linked into libFortranRuntime.so. Eventually this workaround PR shoud be redone and a proper runtime build should be setup that will then also compile Fortran MODULE files.
3f6a963
to
01179b1
Compare
Co-authored-by: Valentin Clement (バレンタイン クレメン) <[email protected]>
Co-authored-by: Michael Kruse <[email protected]>
I have made the requested updates. |
@kkwli @Meinersbur @clementval Ping :-) |
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, with two nits.
Btw, you should remove the draft status so flang category subscribers are notified |
Co-authored-by: Michael Kruse <[email protected]>
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.
LG
This reverts commit 04f52d6.
@@ -169,6 +169,7 @@ set(sources | |||
unit-map.cpp | |||
unit.cpp | |||
utf.cpp | |||
${FORTRAN_MODULE_OBJECTS} |
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.
This breaks some of my builds. I believe this won't work in general unless you make sure that the object file is built before FortranRuntime
target tries to build, i.e. there has to be a target dependency in this CMake file.
I think you can try to reproduce it by deleting the module object file and running make FortranRuntime
- it will fail with something like:
No rule to make target `tools/flang/tools/f18/iso_fortran_env_impl.o', needed by `lib/libFortranRuntime.a'. Stop.
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.
On it. Seems like Ninja understands the dependency, as it has a single build file that see the generated object file in one place and defers the target in the runtime libraries until that object file has been built. This does not seem to be the case for Makefile builds, because they are split across different Makefiles.
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.
Created #99737 to hopefully resolve this. Can you please see if the PR resolves the issue?
And sorry for breaking your builds!
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. No problem, it should fix my builds, and building module_files
target before the complete build was a workaround for me.
…ew (#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 #95388. --------- Co-authored-by: Michael Kruse <[email protected]>
…ble symbols (llvm#95388) Moves definitions of the kind arrays into a Fortran MODULE to not only emit the MOD file, but also compile that MODULE file into an object file. This file is then linked into libFortranRuntime.so. Eventually this workaround PR shoud be redone and a proper runtime build should be setup that will then also compile Fortran MODULE files. Fixes llvm#89403 --------- Co-authored-by: Valentin Clement (バレンタイン クレメン) <[email protected]> Co-authored-by: Michael Kruse <[email protected]>
…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 <[email protected]>
…ble symbols (#95388) Summary: Moves definitions of the kind arrays into a Fortran MODULE to not only emit the MOD file, but also compile that MODULE file into an object file. This file is then linked into libFortranRuntime.so. Eventually this workaround PR shoud be redone and a proper runtime build should be setup that will then also compile Fortran MODULE files. Fixes #89403 --------- Co-authored-by: Valentin Clement (バレンタイン クレメン) <[email protected]> Co-authored-by: Michael Kruse <[email protected]> Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60251666
…ew (#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 #95388. --------- Co-authored-by: Michael Kruse <[email protected]>
Moves definitions of the kind arrays into a Fortran MODULE to not only emit the MOD file, but also compile that MODULE file into an object file. This file is then linked into libFortranRuntime.so.
Eventually this workaround PR shoud be redone and a proper runtime build should be setup that will then also compile Fortran MODULE files.
Fixes #89403