Open bcornille opened 1 month ago
I agree on all counts. The library build is a bit weird. This has been on my todo list, but there's a lot of things that are a higher priority. It would be a while before I could work on it.
I am willing to provide patches for some of these changes.
That would certainly speed up the process. I will happily review and merge PRs that make the build process more idiomatic.
Suggestion Description
The current use of
HIPFORT_
prefixed setting variables is a very non-standard way to effect a number of build options. Overwriting variousCMAKE_
prefixed settings prevents users from using the commonly accepted ways of specifying, compiler, flags, and install directory. This will be confusing for users that are well versed in CMake. The hard-coded default compiler path is also problematic as this does not respect environment variables likeFC
, which is a commonly accepted way to specify a Fortran compiler for most build systems.Suggested changes:
HIPFORT_
prefixed variables as inputs and use them as internal variables only orHIPFORT_COMPILER
should default to being initialized fromCMAKE_Fortran_COMPILER_INIT
HIPFORT_COMPILER_FLAGS
should default to being initialized fromCMAKE_Fortran_FLAGS_INIT
HIPFORT_BUILD_TYPE
should default to being initialized fromCMAKE_BUILD_TYPE
(it could be made to pick release if empty)set(CMAKE_Fortran_FORMAT FREE)
to set source to freeform without relying on users to provide a correct compiler flagCMAKE_Fortran_PREPROCESS
to turn on preprocessing in all source files (available in CMake 3.18)VERSION
in theproject
function to specify version (note that havingHIPFORT_VERSION
as input does not make much sense as the user does not name the versions of the libray, the version should be built into the project by developers)check_fortran_compiler_flag
(since CMake 3.3) for any flags you intend to hardcode. I don't know how ubiquitous the-std
flag is, but the current build system only works with compilers that support specifically-std=f2008
or Cray (which means no LLVMFlang)I am willing to provide patches for some of these changes.
Operating System
No response
GPU
No response
ROCm Component
No response