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

Use CMake abstract build commands #403

Merged
merged 1 commit into from
Dec 5, 2022

Conversation

scivision
Copy link
Contributor

@scivision scivision commented Nov 29, 2022

use CMake abstract build commands for compactness and clarity

@tclune
Copy link
Member

tclune commented Nov 29, 2022

@mathomp4 you will need to review this PR.

@Leonard-Reuter
Copy link
Contributor

I opened a related draft pull request: #404
atm, ifx is not supported by pFUnit (mainly due to the lack of an IntelLLVM.cmake file).

@mathomp4
Copy link
Collaborator

@scivision I am a bit concerned that we go straight from ifort support to no ifort support in the CI. Why not start with two tests, one "Ifort Classic" and the other "Ifort LLVM". I'd prefer to keep Ifort Classic as long as the Intel repos have it around (and even then, we could install an older intel set of compilers as long as it it in the Apt repo).

But there are more issues.

First, every GFE repo needs an IntelLLVM.cmake file. This is "simple" in that we can probably just do a cp Intel.cmake IntelLLVM.cmake and be okay. The flags in GFE are pretty benign.

The bigger issue is that ifx cannot build GFE (pFUnit at least). When I try with ifx 2022.2 I get:

/gpfsm/dulocal/sles12/intel/oneapi/2021/compiler/2022.2.0/linux/bin-llvm/xfortcom[0xeec8a9]

/discover/swdev/mathomp4/GFE-IFX-Try/GFE/pFUnit/src/funit/core/NameFilter.F90(34): error #5533: Feature found on this line is not yet supported in ifx
    filter = index(a_test%getName(), this%pattern) > 0
-------------------^
compilation aborted for /discover/swdev/mathomp4/GFE-IFX-Try/GFE/pFUnit/src/funit/core/NameFilter.F90 (code 3)
pFUnit/src/funit/core/CMakeFiles/funit-core.dir/build.make:698: recipe for target 'pFUnit/src/funit/core/CMakeFiles/funit-core.dir/NameFilter.F90.o' failed

When I see this:

 error #5533: Feature found on this line is not yet supported in ifx

I think this compiler is still too feature-light. I mean, Intel might say:

In this release in oneAPI 2022.2 ifx completely implements Fortran 77, Fortran90/95, Fortran 2003 (including parameterized derived types) and Fortran 2008 (except coarrays) language standards and OpenMP 4.5 and OpenMP 5.0/5.1 directives and offloading features. ifx is binary (.o/.obj) and module file (.mod) compatible.

But, I mean, the compiler seems to disagree! Maybe ifx 2022.3 does support it, but we don't have it on discover at the moment. And I don't think that feature is Fortran 2018 as that was mainly C interoperability and coarray stuff (@tclune can be more precise).

Maybe @tclune can figure out how to workaround the fact that ifx does not have the capabilities of ifort with #ifdef but it's also possible that construct is used a lot in pFUnit and, well, that's a lot of #ifdef because a compiler is missing a Standard feature.

NOTE: I think moving away from icc to icx is doable even with ifort. Once we have IntelLLVM.cmake files, the small amount of C in GFE should be easily doable by icx. It's not like we are using crazy advanced C features.

@mathomp4 mathomp4 mentioned this pull request Nov 29, 2022
@scivision scivision changed the title CI: use oneAPI IntelLLVM. Use CMake abstract build commands Use CMake abstract build commands Nov 30, 2022
@scivision
Copy link
Contributor Author

scivision commented Nov 30, 2022

I removed oneAPI changes from this. I was planning another PR to simplify the compiler options -- these can be handled without overwriting defaults that can break the newer compilers by using add_compile_options() and CMake generator expressions.

For now this just simplifies the CI cmake. Yes oneAPI 2022.2 isn't ready for this advanced level of Fortran syntax yet.

Copy link
Collaborator

@mathomp4 mathomp4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it passed. So seems safe. Approving!

@tclune tclune merged commit a4300e6 into Goddard-Fortran-Ecosystem:develop Dec 5, 2022
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

Successfully merging this pull request may close these issues.

4 participants