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

Add ifx support #404

Merged

Conversation

Leonard-Reuter
Copy link
Contributor

This is a draft pull request, I simply copied Intel.cmake to IntelLLVM.cmake. With ifx (IFORT) 2021.4.0 Beta 20210924 pFUnit does however not compile due to an compilation error in gFTL-shared:

/home/reuter/GitProjects/pFUnit_github/extern/fArgParse/extern/gFTL-shared/src/v1/demo.F90(299): error #6197: An assignment of different structure types is invalid.   [INTEGER32INTEGER32_MAP]
   Integer32Integer_map = Integer32Integer32_map
--------------------------^
compilation aborted for /home/reuter/GitProjects/pFUnit_github/extern/fArgParse/extern/gFTL-shared/src/v1/demo.F90 (code 1)
make[2]: *** [extern/fArgParse/extern/gFTL-shared/src/v1/CMakeFiles/demo.x.dir/build.make:75: extern/fArgParse/extern/gFTL-shared/src/v1/CMakeFiles/demo.x.dir/demo.F90.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:980: extern/fArgParse/extern/gFTL-shared/src/v1/CMakeFiles/demo.x.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....

@mathomp4
Copy link
Collaborator

@Leonard-Reuter You'd probably need to add an IntelLLVM.cmake file to gFTL, gFTL-shared, fArgParse, etc. Essentially all the GFE libraries. I can work with @tclune to start spreading that through GFE.

But as I say in #403 (comment), at least with ifx 2022.2 the compiler is still incomplete.

If you are willing to try it out, you can do:

gh repo clone Goddard-Fortran-Ecosystem/GFE
cd GFE
git submodule update --init

to get the latest GFE. Then you can go into each of the submodules and do the cp Intel.cmake IntelLLVM.cmake to prepare it, then try doing a build of GFE. If ifx 2022.4 beta works, maybe that's a good sign ifx is ready soon.

@scivision
Copy link
Contributor

scivision commented Nov 30, 2022

CC=icx FC=ifort does work:

-- The Fortran compiler identification is Intel 2021.7.0.20220726
-- The C compiler identification is IntelLLVM 2022.2.0

However as noted ifx 2022.2 is not yet ready for pFUnit.

@scivision
Copy link
Contributor

scivision commented Nov 30, 2022

With this patch, ifx 2022.2.0 does build. The only other change I did from pfUnit main branch was copying cmake/Intel.cmake => cmake/IntelLLVM.cmake as above.

diff --git a/src/funit/core/NameFilter.F90 b/src/funit/core/NameFilter.F90
index 8483df4..1f3f3da 100644
--- a/src/funit/core/NameFilter.F90
+++ b/src/funit/core/NameFilter.F90
@@ -30,8 +30,12 @@ contains
   logical function filter(this, a_test)
     class(NameFilter), intent(in) :: this
     class(Test), intent(in) :: a_test
+
+    ! workaround for oneAPI ifx 2022.2.0
+    character(:), allocatable :: name
+    name = a_test%getName()

-    filter = index(a_test%getName(), this%pattern) > 0
+    filter = index(name, this%pattern) > 0
   end function filter


diff --git a/src/funit/fhamcrest/StringContains.F90 b/src/funit/fhamcrest/StringContains.F90
index 127d757..ad23b83 100644
--- a/src/funit/fhamcrest/StringContains.F90
+++ b/src/funit/fhamcrest/StringContains.F90
@@ -35,7 +35,12 @@ contains
     class(StringContains), intent(in) :: this
     character(*), intent(in) :: item

-    eval_substring_of = index(this%converted(item), this%converted(this%get_substring())) > 0
+    ! workaround for Intel oneAPI ifx 2022.2.0
+    character(:), allocatable :: converted, convsub
+    converted = this%converted(item)
+    convsub = this%converted(this%get_substring())
+
+    eval_substring_of = index(converted, convsub) > 0
   end function eval_substring_of

 end module pf_StringContains
diff --git a/src/funit/fhamcrest/StringEndsWith.F90 b/src/funit/fhamcrest/StringEndsWith.F90
index e8f7a31..ce548d1 100644
--- a/src/funit/fhamcrest/StringEndsWith.F90
+++ b/src/funit/fhamcrest/StringEndsWith.F90
@@ -35,8 +35,13 @@ contains
     character(*), intent(in) :: item

     integer :: idx_substring
+
+    ! workaround for Intel oneAPI ifx 2022.2.0
+    character(:), allocatable :: buf1, buf2
+    buf1 = this%converted(item)
+    buf2 = this%converted(this%get_substring())

-    idx_substring = index(this%converted(item), this%converted(this%get_substring()),back=.true.)
+    idx_substring = index(buf1, buf2, back=.true.)

     eval_substring_of = idx_substring == (len(item) - len(this%get_substring()) + 1)
   end function eval_substring_of
diff --git a/src/funit/fhamcrest/StringStartsWith.F90 b/src/funit/fhamcrest/StringStartsWith.F90
index 4caf65b..fb90ef5 100644
--- a/src/funit/fhamcrest/StringStartsWith.F90
+++ b/src/funit/fhamcrest/StringStartsWith.F90
@@ -33,7 +33,13 @@ contains
   logical function eval_substring_of(this, item)
     class(StringStartsWith), intent(in) :: this
     character(*), intent(in) :: item
-    eval_substring_of = index(this%converted(item), this%converted(this%get_substring())) == 1
+
+    ! workaround for Intel oneAPI ifx 2022.2.0
+    character(:), allocatable :: converted, convsub
+    converted = this%converted(item)
+    convsub = this%converted(this%get_substring())
+
+    eval_substring_of = index(converted, convsub) == 1
   end function eval_substring_of

 end module pf_StringStartsWith

Running ctest results in:

1/6 Test #2: robust_tests.x ...................   Passed    0.00 sec
2/6 Test #1: old_tests ........................***Failed    0.00 sec
3/6 Test #4: fhamcrest_tests.x ................***Failed    0.01 sec
4/6 Test #3: new_tests.x ......................   Passed    0.01 sec
5/6 Test #5: mpi-tests ........................***Failed    1.99 sec
6/6 Test #6: new_ptests .......................   Passed    1.99 sec

50% tests passed, 3 tests failed out of 6

Total Test time (real) =   1.99 sec

The following tests FAILED:
          1 - old_tests (Failed)
          4 - fhamcrest_tests.x (Failed)
          5 - mpi-tests (Failed)

With the same patched code, icx + ifort does pass all ctest. The failing ifx ctest is segfaults.

@scivision
Copy link
Contributor

rebuilding using

cmake -Bbuildi -DCMAKE_C_COMPILER=icx -DCMAKE_Fortran_COMPILER=ifx  -DSKIP_MPI=yes -DSKIP_OPENMP=yes -DSKIP_FHAMCREST=yes -DSKIP_ROBUST=yes

the ctest output is then

1/2 Test #2: new_tests.x ......................   Passed    0.01 sec
2/2 Test #1: old_tests ........................   Passed    0.01 sec

100% tests passed, 0 tests failed out of 2

Total Test time (real) =   0.01 sec

@scivision
Copy link
Contributor

scivision commented Nov 30, 2022

This patch helps Intel compiler build by not overriding necessary factory options. Especially on windows, where numerous errors occur on any source file before this patch

diff --git a/cmake/Intel.cmake b/cmake/Intel.cmake
index 3dbbea8..7405fcb 100644
--- a/cmake/Intel.cmake
+++ b/cmake/Intel.cmake
@@ -1,22 +1,21 @@
 # Compiler specific flags for Intel Fortran compiler

+set(check_all -check)
+# check without options == "check all". This helps avoid command line processing issues
+
+set(traceback -traceback)
 if(WIN32)
-  set(no_optimize "-Od")
-  set(check_all "-check:all")
+  set(disable_warning_for_long_names /Qdiag-disable:5462)
 else()
-  set(no_optimize "-O0")
-  set(check_all "-check all")
+  set(disable_warning_for_long_names -diag-disable=5462)
 endif()
-
-
-set(disable_warning_for_long_names "-diag-disable 5462")
-set(traceback "-traceback")
-set(cpp "-cpp")

+set(common_flags "${disable_warning_for_long_names}")

-set(common_flags "${cpp} ${disable_warning_for_long_names}")
-set(CMAKE_Fortran_FLAGS_DEBUG  "-O0 -g ${common_flags} ${traceback} ${no_optimize} ${check_all}")
-set(CMAKE_Fortran_FLAGS_RELEASE "-O3 ${common_flags}")
+add_compile_options(
+"$<$<AND:$<COMPILE_LANGUAGE:Fortran>,$<CONFIG:Debug>>:${traceback};${check_all}>"
+"$<$<COMPILE_LANGUAGE:Fortran>:${common_flags}>"
+)

-add_definitions(-D_INTEL)
-add_definitions(-D__ifort_18)
+add_compile_definitions(_INTEL)
+add_compile_definitions(__ifort_18)

@Leonard-Reuter
Copy link
Contributor Author

Just mentioning the related issue: #336

@scivision
Copy link
Contributor

scivision commented Dec 21, 2022

Build is now successful with Intel 2023.0 and current pfUnit master, just copying Intel.cmake to IntelLLVM.cmake.

However, all tests fail with default configuration.

cmake -Bbuildi
-- The Fortran compiler identification is IntelLLVM 2023.0.0
-- The C compiler identification is IntelLLVM 2023.0.0
cmake --build buildi
ctest --test-dir buildi
1/6 Test #1: old_tests ........................***Failed    0.00 sec
2/6 Test #4: fhamcrest_tests.x ................***Failed    0.00 sec
3/6 Test #3: new_tests.x ......................***Failed    0.01 sec
4/6 Test #2: robust_tests.x ...................Subprocess aborted***Exception:   0.17 sec
5/6 Test #6: new_ptests .......................***Failed    1.97 sec
6/6 Test #5: mpi-tests ........................***Failed    1.98 sec

If I use the following configuration, tests also pass:

cmake  -DSKIP_MPI=yes -DSKIP_OPENMP=yes -DSKIP_FHAMCREST=yes -DSKIP_ROBUST=yes

1/2 Test #2: new_tests.x ...................... Passed 0.01 sec
2/2 Test #1: old_tests ........................ Passed 0.02 sec

@mathomp4 mathomp4 marked this pull request as ready for review April 17, 2023 13:27
@mathomp4 mathomp4 requested a review from tclune as a code owner April 17, 2023 13:27
Copy link
Member

@tclune tclune left a comment

Choose a reason for hiding this comment

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

@mathomp4 Please check this over. If branch is ok, please fetch it and add our ChangeLog updates and such and we'll roll out a release.

@tclune tclune merged commit da2d12f into Goddard-Fortran-Ecosystem:develop Apr 17, 2023
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