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

Testing with C++14 standard #1914

Closed
3 tasks done
ndellingwood opened this issue Nov 28, 2018 · 15 comments
Closed
3 tasks done

Testing with C++14 standard #1914

ndellingwood opened this issue Nov 28, 2018 · 15 comments
Assignees
Labels
Enhancement Improve existing capability; will potentially require voting
Milestone

Comments

@ndellingwood
Copy link
Contributor

ndellingwood commented Nov 28, 2018

@ndellingwood ndellingwood added the Enhancement Improve existing capability; will potentially require voting label Nov 28, 2018
@ndellingwood ndellingwood added this to the 2018 December milestone Nov 28, 2018
ndellingwood added a commit that referenced this issue Nov 28, 2018
@ndellingwood
Copy link
Contributor Author

These nightly tests on White test c++14:
gcc/7.2.0 host builds
gcc/7.2.0+cuda/9.2 Pascal node

c++17 testing:
gcc/7.2.0 host builds

Adding @crtrott @nmhamster @srajama1 @kyungjoo-kim

@nmhamster
Copy link
Contributor

@ndellingwood - should the --cxxstandard work with IBM XL?

This configuration line does not seem to put the standard on the compile line:

../generate_makefile.bash --arch=Power9 --with-openmp --compiler=`which xlC` --cxxstandard="c++14"

@ndellingwood
Copy link
Contributor Author

@nmhamster I thought it should I'll look into it, thanks for checking

@nmhamster
Copy link
Contributor

OK this does work:

../generate_makefile.bash --arch=Power9 --with-openmp --compiler=`which xlC` --cxxstandard="c++14" --cxxflags="-std=c++14"

It seems weird to me that we wouldn't set the CXXFLAGS automatically if the --cxxstandard is given to us.

@ndellingwood
Copy link
Contributor Author

@nmhamster Can you check the KokkosCore_config file in the first case if you didn't blow that build away? When I tested with gcc and cuda it was showing that macro #define KOKKOS_ENABLE_CXX14 was properly defined, if that is true in the first case then that will help pinpoint where the issue is occurring in setting the flags.

@ndellingwood
Copy link
Contributor Author

With gcc I can set cxxstandard=c++14 and it will add the proper cxxflags e.g.

g++ -I./ -I/ascldap/users/ndellin/kokkos/core/src -I/ascldap/users/ndellin/kokkos/containers/src -I/ascldap/users/ndellin/kokkos/algorithms/src -I/ascldap/users/ndellin/kokkos/core/src/eti --std=c++14 -mcpu=power8 -mtune=power8 -I/ascldap/users/ndellin/kokkos/tpls/gtest -I/ascldap/users/ndellin/kokkos/core/unit_test -O3 -c /ascldap/users/ndellin/kokkos/core/unit_test/UnitTestMainInit.cpp

@ndellingwood
Copy link
Contributor Author

Checking this issue with xl now...

@ndellingwood
Copy link
Contributor Author

On White with ibm xl/16.1.0 I run into errors.

Makefile configuration:
module load devpack/20180521/openmpi/2.1.2/ibm/xl/16.1.0/cuda/9.2.88

../../generate_makefile.bash --with-serial --cxxstandard="c++14" --with-cuda-options=enable_lambda --arch="Power8" --prefix=pwd/install

Sample compilation line - c++ standard is missing:
/home/projects/ppc64le/ibm/xlC/16.1.0/bin/xlC -I./ -I/ascldap/users/ndellin/kokkos/core/src -I/ascldap/users/ndellin/kokkos/containers/src -I/ascldap/users/ndellin/kokkos/algorithms/src -I/ascldap/users/ndellin/kokkos/core/src/eti -mcpu=power8 -mtune=power8 -qsmp=omp -O3 -c /ascldap/users/ndellin/kokkos/core/src/impl/Kokkos_ExecPolicy.cpp

But if I generate the makefile without the cxxstandard the --std=c++11 options shows up as expected by default:

/home/projects/ppc64le/ibm/xlC/16.1.0/bin/xlC -I./ -I/ascldap/users/ndellin/kokkos/core/src -I/ascldap/users/ndellin/kokkos/containers/src -I/ascldap/users/ndellin/kokkos/algorithms/src -I/ascldap/users/ndellin/kokkos/core/src/eti -std=c++11 -mcpu=power8 -mtune=power8 -qsmp=omp -O3 -c /ascldap/users/ndellin/kokkos/core/src/impl/Kokkos_ExecPolicy.cpp

@ndellingwood
Copy link
Contributor Author

Here we go, in Makefile.kokkos when checking the XL compiler KOKKOS_INTERNAL_COMPILER_XL, any option for setting the c++ standard variable KOKKOS_INTERNAL_CXX**_FLAG besides c++11 or c++1y was commented out which was why setting cxxstandard did not result in the flag being set correctly.
Issuing PR with the fix now.

@nmhamster thanks for catching this issue!

ndellingwood added a commit that referenced this issue Dec 5, 2018
Part of issue #1914, this allows the XL compiler to properly set
flags for c++14, c++17, etc.
@nmhamster
Copy link
Contributor

nmhamster commented Dec 5, 2018

This is the first set of errors from XL running with -std=c++14 set.

unit_test/openmp/TestOpenMP_AtomicOperations_longint.cpp
In file included from /ascldap/users/sdhammo/git/kokkos-github-repo/core/unit_test/openmp/TestOpenMP_Complex.cpp:46:
In file included from /ascldap/users/sdhammo/git/kokkos-github-repo/core/unit_test/TestComplex.hpp:44:
In file included from /ascldap/users/sdhammo/git/kokkos-github-repo/core/src/Kokkos_Core.hpp:91:
/ascldap/users/sdhammo/git/kokkos-github-repo/core/src/Kokkos_Complex.hpp:590:75: error: no matching member function for call to 'real'
  return complex<typename std::common_type<RealType1,RealType2>::type> (x.real () * y.real () - x.imag () * y.imag (),
                                                                        ~~^~~~
/ascldap/users/sdhammo/git/kokkos-github-repo/core/unit_test/TestComplex.hpp:78:11: note: in instantiation of function template specialization
      'Kokkos::operator*<double, double>' requested here
    r = sb*a; r_kk = b*a;    ASSERT_FLOAT_EQ(r.real(),r_kk.real()); ASSERT_FLOAT_EQ(r.imag(),r_kk.imag());
          ^
/ascldap/users/sdhammo/git/kokkos-github-repo/core/unit_test/TestComplex.hpp:108:8: note: in instantiation of member function
      'Test::TestComplexConstruction<Kokkos::OpenMP>::testit' requested here
  test.testit();
       ^
/usr/lib/gcc/ppc64le-redhat-linux/4.8.5/../../../../include/c++/4.8.5/complex:1220:7: note: candidate function not viable: 'this' argument has type
      'const std::complex<double>', but method is not marked const
      real() { return __real__ _M_value; }
      ^
/usr/lib/gcc/ppc64le-redhat-linux/4.8.5/../../../../include/c++/4.8.5/complex:1242:7: note: candidate function not viable: requires single argument '__val', but no arguments
      were provided
      real(double __val) { __real__ _M_value = __val; }
      ^
/usr/lib/gcc/ppc64le-redhat-linux/4.8.5/../../../../include/c++/4.8.5/complex:1298:29: error: no matching member function for call to 'real'
          __real__ _M_value += __z.real();
                               ~~~~^~~~
...

@nmhamster
Copy link
Contributor

Did a quick spot check GCC 7.2.0 on POWER9 with OpenMP and -std=c++14 looks good.

@nmhamster
Copy link
Contributor

Spot check on GCC 7.2.0 with CUDA 9.2 on POWER9/Volta also looks good.

@ndellingwood
Copy link
Contributor Author

@nmhamster @crtrott using xl/16.1.0 on White seems to be broken with c++14 as the standard.

Simple reproducer:

// complex_add_cpp14.cpp 
#include <iostream>
#include <complex>

int main() {

  std::complex<double> a(1.5,2.5);
  std::complex<double> b(3.25,5.75);

  std::complex<double> r;

  r = a+b;

  std::cout << " r = " << r << std::endl;

 return 0;
}

This will compile with -std=c++11:

xlC complex_add_cpp14.cpp -std=c++11 -lm

Broken with std=c++14:
xlC complex_add_cpp14.cpp -std=c++14 -lm

Errors:

In file included from complex_add_cpp14.cpp:6:
/usr/lib/gcc/ppc64le-redhat-linux/4.8.5/../../../../include/c++/4.8.5/complex:1298:29: error: no matching member function for call to 'real'
          __real__ _M_value += __z.real();
                               ~~~~^~~~
/usr/lib/gcc/ppc64le-redhat-linux/4.8.5/../../../../include/c++/4.8.5/complex:327:11: note: in instantiation of function template specialization
      'std::complex<double>::operator+=<double>' requested here
      __r += __y;
          ^
complex_add_cpp14.cpp:15:8: note: in instantiation of function template specialization 'std::operator+<double>' requested here
  r = a+b;
       ^
/usr/lib/gcc/ppc64le-redhat-linux/4.8.5/../../../../include/c++/4.8.5/complex:1220:7: note: candidate function not viable: 'this' argument has type
      'const complex<double>', but method is not marked const
      real() { return __real__ _M_value; }
      ^
/usr/lib/gcc/ppc64le-redhat-linux/4.8.5/../../../../include/c++/4.8.5/complex:1242:7: note: candidate function not viable: requires single argument '__val', but
      no arguments were provided
      real(double __val) { __real__ _M_value = __val; }
      ^
/usr/lib/gcc/ppc64le-redhat-linux/4.8.5/../../../../include/c++/4.8.5/complex:1299:29: error: no matching member function for call to 'imag'
          __imag__ _M_value += __z.imag();
                               ~~~~^~~~
/usr/lib/gcc/ppc64le-redhat-linux/4.8.5/../../../../include/c++/4.8.5/complex:1224:7: note: candidate function not viable: 'this' argument has type
      'const complex<double>', but method is not marked const
      imag() { return __imag__ _M_value; }
      ^
/usr/lib/gcc/ppc64le-redhat-linux/4.8.5/../../../../include/c++/4.8.5/complex:1245:7: note: candidate function not viable: requires single argument '__val', but
      no arguments were provided
      imag(double __val) { __imag__ _M_value = __val; }
      ^
2 errors generated.

ndellingwood added a commit that referenced this issue Dec 5, 2018
Part of issue #1914, this allows the XL compiler to properly set
flags for c++14
@ndellingwood
Copy link
Contributor Author

@nmhamster @crtrott thanks for helping resolve the include and lib path issues on White to fix the errors in the previous comment.

@ndellingwood
Copy link
Contributor Author

PR #1926 merged with Makefile.kokkos update to properly set cxxflags when cxxstandard=c++14 is set when using XL compiler.

dhollman pushed a commit that referenced this issue Feb 27, 2019
dhollman pushed a commit that referenced this issue Feb 27, 2019
Part of issue #1914, this allows the XL compiler to properly set
flags for c++14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Improve existing capability; will potentially require voting
Projects
None yet
Development

No branches or pull requests

3 participants