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

Replace enums in Kokkos_ViewMapping.hpp #3422

Merged

Conversation

masterleinad
Copy link
Contributor

Alternative to #3420 to prevent some -Wextra warnings regarding comparing enums and non-enums.

@masterleinad
Copy link
Contributor Author

Retest this please.

2 similar comments
@masterleinad
Copy link
Contributor Author

Retest this please.

@masterleinad
Copy link
Contributor Author

Retest this please.

@masterleinad
Copy link
Contributor Author

This seems to work as well.

@djglaze
Copy link

djglaze commented Sep 25, 2020

This is a better solution than what I was proposing. I was under the (mistaken) belief that I would have to add a .cpp file to add definitions for these variables. Auto-inlining from the header solves the problem. I will gladly abandon my PR in place of this if it passes review.

@crtrott
Copy link
Member

crtrott commented Sep 25, 2020

I want some more compiler coverage than what CI does. I tried that before and not all of them ate this. But that was before our switch to requiring C++14 and the associated drop of some compilers. We had CUDA 8 and Intel 15 back then for example. So this might be fine now, and if so I want this solution.

@ndellingwood
Copy link
Contributor

@crtrott @masterleinad I ran a spot-check with a subset of intel compilers + backends on kokkos-dev, no errors/failures.
Note the script changes were to add intel/18 and intel/19 options

[ndellin@kokkos-dev IntelTesting]$ ../../scripts/testing_scripts/test_all_sandia intel --spot-check
Running on machine: kokkos-dev
WARNING!! THE FOLLOWING CHANGES ARE UNCOMMITTED!! :
 M scripts/testing_scripts/test_all_sandia
 M scripts/testing_scripts/update_lib.sh

Repository Status:  5cfafd1af0bf3ccc07559d536cee31f781ee1557 Replace enums in Kokkos_ViewMapping.hpp


Going to test compilers:  intel/17.0.1 intel/18.0.5 intel/19.0.5
Testing compiler intel/17.0.1
Testing compiler intel/18.0.5
  Starting job intel-17.0.1-OpenMP-release
kokkos options: 
kokkos devices: OpenMP
kokkos cxx: -O3 -Wall -Wunused-parameter -Wshadow -pedantic -Werror -Wsign-compare -Wtype-limits -Wuninitialized 
  PASSED intel-17.0.1-OpenMP-release
Testing compiler intel/19.0.5
  Starting job intel-18.0.5-Serial-release
kokkos options: 
kokkos devices: Serial
kokkos cxx: -O3 -Wall -Wunused-parameter -Wshadow -pedantic -Werror -Wsign-compare -Wtype-limits -Wuninitialized 
  PASSED intel-18.0.5-Serial-release
  Starting job intel-19.0.5-Pthread_Serial-release
kokkos options: 
kokkos devices: Pthread,Serial
kokkos cxx: -O3 -Wall -Wunused-parameter -Wshadow -pedantic -Werror -Wsign-compare -Wtype-limits -Wuninitialized 
  PASSED intel-19.0.5-Pthread_Serial-release
#######################################################
PASSED TESTS
#######################################################
intel-17.0.1-OpenMP-release build_time=200 run_time=229
intel-18.0.5-Serial-release build_time=166 run_time=326
intel-19.0.5-Pthread_Serial-release build_time=314 run_time=965

@crtrott
Copy link
Member

crtrott commented Sep 25, 2020

Ok cool

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.

None yet

4 participants