Skip to content
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

[Feature]: Accept standard CMake input conventions #176

Open
bcornille opened this issue Jul 17, 2024 · 1 comment
Open

[Feature]: Accept standard CMake input conventions #176

bcornille opened this issue Jul 17, 2024 · 1 comment
Assignees

Comments

@bcornille
Copy link
Contributor

Suggestion Description

The current use of HIPFORT_ prefixed setting variables is a very non-standard way to effect a number of build options. Overwriting various CMAKE_ 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 like FC, which is a commonly accepted way to specify a Fortran compiler for most build systems.

Suggested changes:

  • Drop all HIPFORT_ prefixed variables as inputs and use them as internal variables only or
    • HIPFORT_COMPILER should default to being initialized from CMAKE_Fortran_COMPILER_INIT
    • HIPFORT_COMPILER_FLAGS should default to being initialized from CMAKE_Fortran_FLAGS_INIT
    • HIPFORT_BUILD_TYPE should default to being initialized from CMAKE_BUILD_TYPE (it could be made to pick release if empty)
  • Use set(CMAKE_Fortran_FORMAT FREE) to set source to freeform without relying on users to provide a correct compiler flag
  • Use CMAKE_Fortran_PREPROCESS to turn on preprocessing in all source files (available in CMake 3.18)
  • Use VERSION in the project function to specify version (note that having HIPFORT_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)
  • Please use 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

@cgmb cgmb self-assigned this Jul 17, 2024
@cgmb
Copy link
Collaborator

cgmb commented Jul 17, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants