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

Fix compiler flags for PGI/NVHPC #4264

Merged
merged 1 commit into from
Aug 26, 2021
Merged

Conversation

masterleinad
Copy link
Contributor

@masterleinad masterleinad commented Aug 25, 2021

-tp=host and native are not accepted by pgc++ 20.1 (only -tp=pwr8 and -tp=pwr9) and nvc++ accepts -tp=host, -tp=native, -tp=host, -tp=pwr8 and -tp=pwr9. Thus, using -tp=pwr8 and -tp=pwr9 seems to be the best compromise. I don't know if these flags are available for older PGI versions (and if we care if not).
Also, there was another branch where we need to replace PGI by NVHPC in our CMake setup.

@masterleinad masterleinad added the Blocks Promotion Overview issue for release-blocking bugs label Aug 25, 2021
@masterleinad masterleinad added this to In progress in Kokkos Release 3.5 via automation Aug 25, 2021
@masterleinad masterleinad added this to Awaiting Feedback in Developer: Daniel Arndt Aug 25, 2021
@masterleinad masterleinad moved this from In progress to Awaiting Feedback in Kokkos Release 3.5 Aug 25, 2021
@DavidPoliakoff
Copy link
Contributor

Does this need to be reflected in the Makefile system?

@dalg24
Copy link
Member

dalg24 commented Aug 26, 2021

In our README we say we support 18.7

* PGI 18.7

although we only enforce it is at least 17.4 in cmake
SET(KOKKOS_MESSAGE_TEXT "${KOKKOS_MESSAGE_TEXT}\n PGI 17.4 or higher\n")

and in our code
#if (1740 > KOKKOS_COMPILER_PGI)

According the 18.7 documentation does not list pwr8 nor pwr9
https://docs.nvidia.com/hpc-sdk/pgi-compilers/18.7/x86/pgi-ref-guide/index.htm#tp
Neither does the 20.1 doc even though it is there on Summit. Oh fun...

@dalg24
Copy link
Member

dalg24 commented Aug 26, 2021

Does this need to be reflected in the Makefile system?

diff --git a/Makefile.kokkos b/Makefile.kokkos
index ace24d63f..fe0e055f1 100644
--- a/Makefile.kokkos
+++ b/Makefile.kokkos
@@ -910,7 +910,8 @@ ifeq ($(KOKKOS_INTERNAL_USE_ARCH_POWER8), 1)
   tmp := $(call kokkos_append_header,"$H""define KOKKOS_ARCH_POWER8")
 
   ifeq ($(KOKKOS_INTERNAL_COMPILER_PGI), 1)
-
+        KOKKOS_CXXFLAGS += -tp=pwr8
+        KOKKOS_LDFLAGS  += -tp=pwr8
   else
     ifeq ($(KOKKOS_INTERNAL_COMPILER_XL), 1) 
         KOKKOS_CXXFLAGS += -mcpu=power8 -mtune=power8
@@ -931,7 +932,8 @@ ifeq ($(KOKKOS_INTERNAL_USE_ARCH_POWER9), 1)
   tmp := $(call kokkos_append_header,"$H""define KOKKOS_ARCH_POWER9")
 
   ifeq ($(KOKKOS_INTERNAL_COMPILER_PGI), 1)
-
+        KOKKOS_CXXFLAGS += -tp=pwr9
+        KOKKOS_LDFLAGS  += -tp=pwr9
   else
     ifeq ($(KOKKOS_INTERNAL_COMPILER_XL), 1) 
         KOKKOS_CXXFLAGS += -mcpu=power9 -mtune=power9

@@ -173,6 +173,8 @@ ENDIF()

IF(NOT DEFINED KOKKOS_CXX_HOST_COMPILER_ID)
SET(KOKKOS_CXX_HOST_COMPILER_ID ${KOKKOS_CXX_COMPILER_ID})
ELSEIF(KOKKOS_CXX_HOST_COMPILER_ID STREQUAL PGI)
SET(KOKKOS_CXX_HOST_COMPILER_ID NVHPC CACHE STRING INTERNAL FORCE)
Copy link
Member

Choose a reason for hiding this comment

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

This change is non obvious to me. Can you elaborate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ideas was to unify PGI and NVHPC so that we don't need to distinguish between different CMake versions. The idea was that nvc++ should accept the same flags as pgc++.
We already did this translation a few lines higher if KOKKOS_CXX_COMPILER_ID was PGI. This didn't account for the case that KOKKOS_CXX_COMPILER_ID was NVIDIA and KOKKOS_CXX_HOST_COMPILER_ID was PGI. This line fixes it.

@masterleinad
Copy link
Contributor Author

Does this need to be reflected in the Makefile system?

We should agree if we want to add that or not. In the corresponding Slack discussion, it was said that the compiler will optimize for the host compiler by default and thus this compiler option is only really relevant for cross-compilation.

Copy link
Member

@dalg24 dalg24 left a comment

Choose a reason for hiding this comment

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

OK looks good to me regardless of the makefile support

@crtrott crtrott merged commit 28a01a0 into kokkos:develop Aug 26, 2021
Kokkos Release 3.5 automation moved this from Awaiting Feedback to Done Aug 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocks Promotion Overview issue for release-blocking bugs
Projects
Developer: Daniel Arndt
Awaiting Feedback
Development

Successfully merging this pull request may close these issues.

None yet

4 participants