core icon indicating copy to clipboard operation
core copied to clipboard

mdsUgrid.cc breaks build on XL compiler

Open jacobmerson opened this issue 5 years ago • 4 comments

This code uses c++11 features which are not available in the XL compiler suite.

Is there a preferred method we are using to only enable certain features if the appropriate compilers/language features exist?

jacobmerson avatar Mar 23 '20 21:03 jacobmerson

For the specific case of XL I might be able to use qlanglvl to fix the error (will test shortly), however we should have a uniform way of dealing with code which requires certain compiler versions.

jacobmerson avatar Mar 23 '20 23:03 jacobmerson

This was on AiMOS?

cwsmith avatar Mar 24 '20 12:03 cwsmith

I was compiling on summit, however I suspect the problem will be encountered anytime you use a compiler which doesn't have the c++11 standard enabled.

jacobmerson avatar Mar 25 '20 03:03 jacobmerson

On AIMOS, I built a minimal install of core with the following commands:

module load xl_r/16.1.1 spectrum-mpi
export OMPI_CXX=xlc++_r
export OMPI_CC=xlc_r

cmake3 ../core -DCMAKE_CXX_COMPILER=mpicxx -DCMAKE_C_COMPILER=mpicc

As described, this fails with:

[ 24%] Building CXX object mds/CMakeFiles/mds.dir/mdsUgrid.cc.o
cd /gpfs/u/home/MPFS/MPFSsmth/barn/build-core-dcs-xlSpectrum/mds && /opt/ibm/spectrum_mpi/bin/mpicxx -+  -I/gpfs/u/home/MPFS/MPFSsmth/barn/build-core-dcs-xlSpectrum/mds -I/gpfs/u/home/MPFS/MPFSsmth/barn/core/pcu -I/gpfs/u/home/MPFS/MPFSsmth/barn/core/pcu/reel -I/gpfs/u/home/MPFS/MPFSsmth/barn/core/gmi -I/gpfs/u/home/MPFS/MPFSsmth/barn/core/lion -I/gpfs/u/home/MPFS/MPFSsmth/barn/core/apf -I/gpfs/u/home/MPFS/MPFSsmth/barn/core/can -I/gpfs/u/home/MPFS/MPFSsmth/barn/core/mth  -O2 -g    -o CMakeFiles/mds.dir/mdsUgrid.cc.o -c /gpfs/u/home/MPFS/MPFSsmth/barn/core/mds/mdsUgrid.cc
/gpfs/u/home/MPFS/MPFSsmth/barn/core/mds/mdsUgrid.cc:319:35: error: expected expression
                                  [upward_dim](const int i) {
                                  ^
/gpfs/u/home/MPFS/MPFSsmth/barn/core/mds/mdsUgrid.cc:318:28: error: no member named 'all_of' in namespace 'std'
      bool same_dim = std::all_of(upward_dim.begin(), upward_dim.end(), 
                      ~~~~~^

Digging around the code I see a few other places where C++11 is used:

$ find . -name \*.cc | xargs grep -ri 'auto\|all_of' | grep = | awk '{print $1}' | sort -u
./ma/maInput.cc:
./mds/mdsUgrid.cc:
./omega_h/apfOmega_h.cc:
./stk/apfSTK.cc:
./test/osh2smb.cc:
./test/smb2osh.cc:

We could add the necessary c++11 flags to each specific target with the following syntax:

$ git diff mds/CMakeLists.txt 
diff --git a/mds/CMakeLists.txt b/mds/CMakeLists.txt
index 785246c..186325b 100644
--- a/mds/CMakeLists.txt
+++ b/mds/CMakeLists.txt
@@ -40,6 +40,13 @@ set(HEADERS
 # Add the mds library
 add_library(mds ${SOURCES})
 
+# Requires C++11
+set_target_properties(mds PROPERTIES
+  CXX_STANDARD 11
+  CXX_STANDARD_REQUIRED ON
+  CXX_EXTENSIONS OFF
+)
+
 # Include directories
 target_include_directories(mds INTERFACE
     $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>

But, since there are multiple places where C++ is used, I'm thinking we should make C++11 required for all of core. The changes are here:

https://github.com/SCOREC/core/commit/8366ee13f60a3f0975d1f4f7bda1d2c2cda125da

Note, there are ctest failures when building with xl and spectrum that need to be examined before this can be merged. I'm also a bit concerned about breaking XSDK support and the other major compilers. This is somewhat of a radical change.

cwsmith avatar Mar 26 '20 14:03 cwsmith