-
-
Notifications
You must be signed in to change notification settings - Fork 306
Fix cross compiling without emulation #5718
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
Fix cross compiling without emulation #5718
Conversation
Didn't meant to ping you all, i meant to mark it as "review" |
2134914
to
69854c9
Compare
This is being tested in conda-forge/hdf5-feedstock#268
69854c9
to
e3ca87c
Compare
if(CMAKE_CROSSCOMPILING) | ||
set(H5MATCH_TYPES_COMPILER "$ENV{H5MATCH_TYPES_COMPILER}" CACHE STRING "compiler for build system") | ||
if (H5MATCH_TYPES_COMPILER) | ||
set_target_properties(H5match_types PROPERTIES | ||
C_COMPILER ${H5MATCH_TYPES_COMPILER} | ||
) | ||
message(STATUS "Using build compiler ${H5MATCH_TYPES_COMPILER} for H5match_types") | ||
endif() | ||
endif() | ||
target_include_directories (H5match_types PRIVATE "${HDF5_SRC_BINARY_DIR};${HDF5_SRC_INCLUDE_DIRS};${HDF5_F90_BINARY_DIR};$<$<BOOL:${HDF5_ENABLE_PARALLEL}>:${MPI_C_INCLUDE_DIRS}>") | ||
|
||
add_custom_command (TARGET H5match_types POST_BUILD | ||
BYPRODUCTS ${HDF5_F90_BINARY_DIR}/H5f90i_gen.h ${HDF5_F90_BINARY_DIR}/H5fortran_types.F90 | ||
COMMAND ${CMAKE_CROSSCOMPILING_EMULATOR} $<TARGET_FILE:H5match_types> | ||
COMMAND $<TARGET_FILE:H5match_types> |
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 whole change is bogus and doesn't work.
We had a similar patch for autotools that had the concept of CC_FOR_BUILD
(man don't get me started about the distinction between build and host and target).
But I can't find any documentation where CMake makes this explicit distinction.
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.
https://cmake.org/cmake/help/book/mastering-cmake/chapter/Cross%20Compiling%20With%20CMake.html has info on cross compile and toolchain files
|
||
include (CheckFortranFunctionExists) | ||
|
||
# Cross-compilation support variables |
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.
Instead of this, let's use the method we used in 1.12 releases to allow the use of a pre-generated 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.
why? a pre-generated file is notoriously difficult to make.
These "overrides" mock the build system early, and ensure more "bug" compatibility, and create a straightforward path for generating these on any machine.
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.
The suggested way, of using cmake variables, is often a common "trope" in CMake.
- Preference for auto detection of capabilities.
- A defined variable as an override.
all the FindPackages
do this kind of stuff, as well as compilers, and other aspects of cmake.
Why should this be any different?
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.
The same process is used. run a configure on an actual target (or create a file) to get the values you need, then supply the values for cross-compile. Read one file to supply the values or supply the values one-by-one in env values. Either way you need the values. The configure is going to create a file for Fortran to use anyway. Personally, a file is easier to deal with, which could be checked in as I don't think there are that many values that will change, Plus we only need to keep the necessary values.
While you have listed all the variables to be used, there are secondary variables that must be set to indicate not to rerun checks that need to be set. From CMake doc: |
I think a CMake toolchain file with the values and the variables to prevent configure runs should work. Instead of random pre-generated file or a list of ENV variables. It would be more CMake like and allow also for some simple logic if needed. |
I guess i routinely read the CMake documentation and come out more confused than when I start. I've been reading it for 15+years and this hasn't really changed. The documentation is "templated" with many "placeholders" that add indirection to this example. I would love it if this section wasn't rerun every time, I think I've noticed the symptoms of it rerunning, but I figured they were harmless. I strongly believe we should avoid "pre-generated" files, but I defer to whatever implementation you want to go with. Getting an example from you, in the form of a draft PR or patch against the |
Yes, I'm working on that. I need to relearn the steps, and figure out the actual variables we do need. Let's table my pregen file idea and see if we can use the toolchain idea first. I will review your list of variables and first create a toolchain file, which target system do you want to start with? See the toolchains in config/toolchain and see if there is one we can copy from. |
Also, we will need to figure out the actual values, if your target system matches one we already natively create - I can use the native values that build. |
I think the challenge is that I want to target a build system that is quite popular outside the high performance compute space. I really want to target:
This combination cannot be "emulated" so any "emulation tools" just fail, which creates a great "sentinel" for any holes in the system. However, i will find
as an acceptable substitute if you can show that the toolchain file variables are being used as an override at cmake configuration time. |
Working on that - Need to do some digging |
lol |
You know, its things like this, with the CMake documentation templating that get me really confused. Then the question is: is it acceptable to rewrite all the fortran detect code...... |
YES. That is exactly my intention in order to use toolchain properly. The other choice is to use toolchain and skip the Fortran try-runs globally by using if(CMAKE_CROSSCOMPILING) CMAKE_CROSSCOMPILING is one of the side-effects of using toolchain files. |
That may be a quick compromise - still need to create a toolchain file. |
include (CheckFortranFunctionExists) | ||
|
||
# Cross-compilation support variables | ||
set(HDF5_FORTRAN_VALID_INT_KINDS "$ENV{HDF5_FORTRAN_VALID_INT_KINDS}" CACHE STRING "Valid Fortran INTEGER kinds (comma-separated)") |
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.
So these names are the H5fort_type_defines.h names not the configure variables. We need to set the configure variables (either in HDF5UseFortran.cmake or in a file)
#define H5_FORTRAN_NATIVE_INTEGER_KIND @PAC_FORTRAN_NATIVE_INTEGER_KIND@
#define H5_FORTRAN_NATIVE_INTEGER_SIZEOF @PAC_FORTRAN_NATIVE_INTEGER_SIZEOF@
#define H5_FORTRAN_NATIVE_REAL_KIND @PAC_FORTRAN_NATIVE_REAL_KIND@
#define H5_FORTRAN_NATIVE_REAL_SIZEOF @PAC_FORTRAN_NATIVE_REAL_SIZEOF@
#define H5_FORTRAN_NATIVE_DOUBLE_KIND @PAC_FORTRAN_NATIVE_DOUBLE_KIND@
#define H5_FORTRAN_NATIVE_DOUBLE_SIZEOF @PAC_FORTRAN_NATIVE_DOUBLE_SIZEOF@
#define H5_FORTRAN_NUM_INTEGER_KINDS @PAC_FORTRAN_NUM_INTEGER_KINDS@
#define H5_FORTRAN_INTEGER_KINDS @PAC_FC_ALL_INTEGER_KINDS@
#define H5_FORTRAN_INTEGER_KINDS_SIZEOF @PAC_FC_ALL_INTEGER_KINDS_SIZEOF@
#define H5_FORTRAN_REAL_KINDS @PAC_FC_ALL_REAL_KINDS@
#define H5_FORTRAN_REAL_KINDS_SIZEOF @PAC_FC_ALL_REAL_KINDS_SIZEOF@
#define H5_HAVE_Fortran_INTEGER_SIZEOF_16 @H5_HAVE_Fortran_INTEGER_SIZEOF_16@
#define H5_FORTRAN_HAVE_C_LONG_DOUBLE @H5_FORTRAN_HAVE_C_LONG_DOUBLE@
#define H5_FORTRAN_C_LONG_DOUBLE_IS_UNIQUE @H5_FORTRAN_C_LONG_DOUBLE_IS_UNIQUE@
PAC_FORTRAN_NATIVE_INTEGER_KIND instead H5_FORTRAN_NATIVE_INTEGER_KIND
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 would really like to keep this PR as a "reference" since it is the one I'm including in the conda-forge PR, but happy to consider an other solution and drop this PR in its entirety.
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.
No problem, I'm using it mostly as communication - I intend to create a PR or to post concrete solutions..
#5720 PR has anoter solution for the HDF5UseFortran file. |
#5720 incorporates this issue |
Closes #5709
H5match_types
. Truthfully, this is likely better written in a scripting language... but i think this workaround is fine.See progress in the OSX ARM builds conda-forge/hdf5-feedstock#268
Important
Enhance HDF5 cross-compilation by using
CC_FOR_BUILD
forH5match_types
and adding Fortran constant overrides.CC_FOR_BUILD
forH5match_types
infortran/src/CMakeLists.txt
to support cross-compilation.config/HDF5UseFortran.cmake
to bypass runtime tests.release_docs/INSTALL_CMake.txt
to include instructions for Fortran cross-compilation support, detailing environment variables and CMake arguments for setting Fortran constants.This description was created by
for ad23ff3. You can customize this summary. It will automatically update as commits are pushed.