[test-suite] SPEC2017 CPU pop2 floating point test.

[naromero77]

Summary

Add the pop2 speed test:

• Workaround for deeply nested subdirectories.
• Add support for the specpp custom preprocessor.
• Set portability flag variables in SpecCPU2017.cmake that are later passed into specpp.
• Guard against running test on little-endian system with a compiler that does not support big-endian binary IO.
• Requires https://reviews.llvm.org/D99619

[Meinersbur]

External/SPEC/CFP2017speed/628.pop2_s/CMakeLists.txt

13

13–16

How does the SPEC .pm driver handle this?

Did you consider invoking speccpu2017_prepare_rundir and just fix the file name after that (Instead of copy-pasting its code)?

spec2017_prepare_rundir()
foreach (_runtype IN LISTS TEST_SUITE_RUN_TYPE)
  llvm_copy(${PROG} "${RUN_${_runtype}_DIR}/drv_in" "${INPUT_${_runtype}_DIR}/drv_in.in")
endforeach ()

36

[typo] note

38

The value of SPEC_AUTO_BYTEORDER depends on the host machine. Some other are added as well. Not sure it makes sense to list the generic SPEC definitions here.

67

Do I understand correctly, gfortran has problems to compile long lines? Is this a problem with the vanilla SPEC as well if installed in a deeply nested directory?

84–85

It is kind-of intransparent where SPEC_AUTO_BYTEORDER_VALUE and SPEC_PTR_TYPE come from. Did you consider assemble a list preprocessor definitions shared between all CPU2017 benchmarks and make speccpu2017_run_specpp add them implicitly (Liek they are added implicitly for the source files themselves)? Similaryl, -U__FILE__ and -w don't seem to be specific to pop2.

106

If it works without IGNORE_WHITESPACE, is it even needed?

110

Is listing the files needed (e.g. because some files must not linked into the executable) or could it also be done with a file(GLOB_RECURSIVE ...)?

438

Could this somehow be shared with 527.cam4_r, e.g. by moving both into SpecCPU2017.cmake?

459

speccpu2017_verify_output will have been called already at this point, only pop2_prepare_rundir is skipped when returning in this point, i.e. it will still try to run pop2 and fail because of files missing. Move this check to the beginning?

External/SPEC/SpecCPU2017.cmake

367

Could explain here why running specpp is needed (instead of the Fortran-compiler's integrated preprocessor)?

370–372

Could use GLOB_RECURSIVE RELATIVE {SRC_DIR} or file(RELATIVE_PATH) instead of get_filename_component to get a relative path. Might need file(MAKE_DIRECTORY if specpp doesn't create directories itself.

This assumes that there even are files in subdirectories that need to be processed.

378

[Idea] Use of target_sources to add the file to the executable instead of needing to list it in speccpu2017_add_executable

Files that need to go trough specpp could also be specified as a "SPECPP_SOURCES" argument of speccpu2017_add_executable and share the preprocessor definitions.

[naromero77]

External/SPEC/CFP2017speed/628.pop2_s/CMakeLists.txt

13–16

There is extra perl code to hand it. Sorry, for the duplicated code. I will fix it.

67

Correct

It should also happen with plain vanilla SPEC, but you would have to intentionally create a deeply nested directory.

84–85

I would have to do some analysis of the other tests like cam4 and wrf to figure it out, but yes, it can be done. And would avoid some duplicated code. Thanks for the suggestion.

106

hmmm... it might a difference with the older version of the floating point comparison utility.

We can take it out.

110

This was quite tricky for me because I wanted to use file(GLOB_RECURSIVE ...), but ran into a lot of trouble on several fronts.

The first issue was:
file(GLOB_RECURSIVE ..) needs to run after you generate the preprocessed files with speccpu2017_run_specpp, not before. So, in my first attempt, I seem to create a race condition due to some dependency not being properly created.

The second issue was:
There is a bunch of files that have *.f90 that are include in other Fortran files and that should not be compiled.

Lastly, and perhaps I do not understand the CMake docs here:
https://cmake.org/cmake/help/latest/command/file.html

It actually advises not to use GLOB to list source files:

We do not recommend using GLOB to collect a list of source files from your source tree. If no CMakeLists.txt file changes when a source is added or removed then the generated build system cannot know when to ask CMake to regenerate. The CONFIGURE_DEPENDS flag may not work reliably on all generators, or if a new generator is added in the future that cannot support it, projects using it will be stuck. Even if CONFIGURE_DEPENDS works reliably, there is still a cost to perform the check on every rebuild.

438

Good idea.

459

I can fix that.

External/SPEC/SpecCPU2017.cmake

367

Yes, I can add that.

370–372

We could, but I think this will turn out to make no difference.

I think that the files that need to be processed by specpp are in the top level directory and not any any subdirectories. The only subdirectory in the SPEC tests that require specpp are the netcdf subdirectory which doesn't actually need specpp.

Even so, maybe it would be good to just code up the general case if its not too hard. At the very least, I can update the comment. But I need to take a look at the other tests.

378

Can

llvm_test_executable(${PROG} ${_sources})

be called multiple times? This is called inside of speccpu2017_add_executable and defines the target ${PROG} and then adds the sources. I think the answer is no, that it cannot be called multiple times -- instead, you are suggesting to use target_sources to append the sources to the already defined target, ${PROG} ?

I will give it a shot.

[Meinersbur]

External/SPEC/CFP2017speed/628.pop2_s/CMakeLists.txt

106

Does D99619 always ignore whitespace? That would be a bug.

110

First issue:
Can glob the unprocessed file names and derive the processed paths using get_filename_component. With the suggested combining of speccpu2017_add_executable and speccpu2017_run_specpp. this would be a single step.

Second issue:
Indeed a problem and why I also fell back to listing all source files explictly in some of the C/C++ benchmarks.

This issue:
It is indeed policy for llvm-project to list all files explicitly for this reason, but llvm-test-suite does this everywhere already and the benchmark source files are not expected to change for SPEC.

External/SPEC/SpecCPU2017.cmake

378

llvm_test_executable cannot be called multiple times.

If merging speccpu2017_add_executable and speccpu2017_run_specpp as suggested in the second paragraph, the list of source files can also be assembled first, then llvm_test_executable called using that list.

[naromero77]

• Use GLOB in speccpu2017_run_specpp
• Move common definitions and flags used by specpp into SpecCPU2017.cmake
• Re-use speccpu2017_prepare_rundir
• Number of minor fixes
• Number of updates to comments

[naromero77]

Trying to send commits again.

[naromero77]

Was still missing earlier commits.

[naromero77]

• big-endian flag fix for GCC and Intel.

[Meinersbur]

I think I would still prefer a common list of compile definitions for specpp and add_definitions. I.e.

set(SPEC_COMMON_DEFS)
if (ENDIAN STREQUAL "little")
  list(APPEND SPEC_COMMON_DEFS "-DSPEC_AUTO_BYTEORDER=0x12345678")
elseif (ENDIAN STREQUAL "big")
  list(APPEND SPEC_COMMON_DEFS "-DSPEC_AUTO_BYTEORDER=0x87654321")
endif ()

[...]

add_definitions(${SPEC_COMMON_DEFS})

[...]

add_custom_command(
  COMMAND "${_specpp_bin}" ${SPEC_COMMON_DEFS} ...
  )

External/SPEC/CFP2017speed/628.pop2_s/CMakeLists.txt

162

What does the SHELL: do?

External/SPEC/SpecCPU2017.cmake

118–125

130–145

375

[change request] This is specific to pop2, i.e. should belong into 628.pop2_s/CMakeLists.txt

Suggestion: Pass a SOURCES option to speccpu2017_run_specpp, do the globbing in 628.pop2_s/CMakeLists.txt

[naromero77]

• Use a single shared variable for definitions that are used by specpp and regular cmake add_definitions function.
• speccpu2017_run_specpp now takes two mandatory lists. one for files and the other for compiler definitions.
• Add missing '>' to generator expression.
• Glob pop2 source Fortran90 files that are passed into specpp in the pop2 test specific CMakeLists.txt.

[Meinersbur]

LGTM, thank you.

External/SPEC/CFP2017speed/628.pop2_s/CMakeLists.txt

+# https://www.spec.org/cpu2017/Docs/benchmarks/628.pop2_s.html
+include(../../SpecCPU2017.cmake)
+if (NOT TEST_SUITE_SPEC2017_ROOT)
+  return ()
+endif ()
+
+ninja_required()
+
+speccpu2017_benchmark(SPEED)
+
+# Check if compiler can support big-endian IO format, if true proceed
+# otherwise, return immediately. This is due to some binary input
+# files stored in big-endian format
+if (ENDIAN STREQUAL "little")
+  # GCC
+  check_fortran_compiler_flag("-fconvert=big-endian" SUPPORTS_FCONVERT_BIG_ENDIAN)
+  # Intel
+  check_fortran_compiler_flag("-convert big_endian" SUPPORTS_CONVERT_BIG_ENDIAN)
+  # Flang
+  # TBD
+  if (NOT SUPPORTS_FCONVERT_BIG_ENDIAN)
+  elseif (NOT SUPPORTS_CONVERT_BIG_ENDIAN)
+  else ()
+    message(WARN "628.pop2_s not supported. No way to read big-endian binary IO on little-endian architecture.")
+    return ()
+  endif ()
+endif ()
+
+macro(pop2_prepare_rundir)
+  # Re-use the speccpu2017_prepare_rundir but then implement an input
+  # file fix at the end. input file for pop2 is hard-coded to drv_in,
+  # but actual filename in SPEC2017 benchmark input directory is
+  # drv_in.in. The SPEC perl run script renames the file as well.
+  speccpu2017_prepare_rundir()
+  foreach (_runtype IN LISTS TEST_SUITE_RUN_TYPE)
+    llvm_copy(${PROG} "${RUN_${_runtype}_DIR}/drv_in" "${INPUT_${_runtype}_DIR}/drv_in.in")
+  endforeach ()
+endmacro()
+
+speccpu2017_add_include_dirs(
+  ${CMAKE_CURRENT_BINARY_DIR}
+  ${SRC_DIR}
+  ${SRC_DIR}/netcdf/include
+)
+
+# pop2 has Fortran sources that need to be processed with specpp and C
+# sources that do not require it.
+
+# Compiler definitions affect C and *.f90 files, but not *.F90 files.
+add_definitions(
+  -D_MPISERIAL
+  -D_NETCDF
+  -D_USEBOX
+  -DCCSMCOUPLED=1
+  -DNO_SHR_VMATH
+  -DBLCKX=50
+  -DBLCKY=4
+  -DMXBLCKS=58
+  -DNO_GETTIMEOFDAY
+  -DSPEC_CASE_FLAG
+)
+
+
+## test ########################################################################
+
+speccpu2017_run_test(RUN_TYPE test)
+
+## train #######################################################################
+
+speccpu2017_run_test(RUN_TYPE train)
+
+## ref #########################################################################
+
+speccpu2017_run_test(RUN_TYPE ref)
+
+################################################################################
+
+# SPEC Test has both absolute and relative tolerance with same order
+speccpu2017_verify_output(IGNORE_WHITESPACE ABSOLUTE_TOLERANCE 0.03 RELATIVE_TOLERANCE 0.03)
+
+# *.F90 files are preprocessed with specpp and written to the CMake build
+# directory. We only list C sources files and Fortran files that don't
+# need to be preprocessed below.
+speccpu2017_add_executable(
+  netcdf/attr.c
+  netcdf/dim.c
+  netcdf/error.c
+  netcdf/fort-attio.c
+  netcdf/fort-control.c
+  netcdf/fort-dim.c
+  netcdf/fort-genatt.c
+  netcdf/fort-geninq.c
+  netcdf/fort-genvar.c
+  netcdf/fort-lib.c
+  netcdf/fort-misc.c
+  netcdf/fort-v2compat.c
+  netcdf/fort-var1io.c
+  netcdf/fort-varaio.c
+  netcdf/fort-vario.c
+  netcdf/fort-varmio.c
+  netcdf/fort-varsio.c
+  netcdf/libvers.c
+  netcdf/nc.c
+  netcdf/ncx.c
+  netcdf/posixio.c
+  netcdf/putget.c
+  netcdf/string.c
+  netcdf/v1hpg.c
+  netcdf/v2i.c
+  netcdf/var.c
+  netcdf/netcdf.f90
+  netcdf/typeSizes.f90
+  mpi.c
+  send.c
+  recv.c
+  collective.c
+  req.c
+  list.c
+  handles.c
+  comm.c
+  group.c
+  time.c
+  pack.c
+  get_zeits.c
+  pnetcdfversion.c
+  topology.c
+  f_wrappers.c
+  GPTLget_memusage.c
+  GPTLprint_memusage.c
+  GPTLutil.c
+  shr_jlcp.c
+  shr_vmath_fwrap.c
+  threadutil.c
+)
+
+# Preprocess *.F90 with specpp and add sources to target
+speccpu2017_run_specpp(
+  -D_MPISERIAL
+  -D_NETCDF
+  -D_USEBOX
+  -DCCSMCOUPLED=1
+  -DNO_SHR_VMATH
+  -DBLCKX=50
+  -DBLCKY=4
+  -DMXBLCKS=58
+  -DNO_GETTIMEOFDAY
+  -I${SRC_DIR}/netcdf/include
+)
+
+
+# Several compiler options below for C and Fortran
+set_property(TARGET ${PROG} PROPERTY C_STANDARD 99)
+
+if (SUPPORTS_FALLOW_ARGUMENT_MISMATCH)
+  target_compile_options(${PROG} PRIVATE $<$<COMPILE_LANGUAGE:Fortran>:-fallow-argument-mismatch>)
+endif ()
+
+if (SUPPORTS_FCONVERT_BIG_ENDIAN) # GCC
+  target_compile_options(${PROG} PRIVATE -fconvert=big-endian)
+elseif (SUPPORTS_CONVERT_BIG_ENDIAN) # Intel
+  target_compile_options(${PROG} PRIVATE -convert big_endian)
+endif ()
+
+pop2_prepare_rundir()

External/SPEC/CFP2017speed/CMakeLists.txt

 if (TEST_SUITE_FORTRAN)
   add_subdirectory(603.bwaves_s)    # Fortran
   add_subdirectory(607.cactuBSSN_s) # C++, C, Fortran
 endif ()
 add_subdirectory(619.lbm_s)        # C
 if (TEST_SUITE_FORTRAN)
   #add_subdirectory(621.wrf_s)     # Fortran, C
   add_subdirectory(627.cam4_s)     # Fortran, C
-  #add_subdirectory(628.pop2_s)    # Fortran, C
+  add_subdirectory(628.pop2_s)     # Fortran, C
 endif ()
 add_subdirectory(638.imagick_s)    # C
 add_subdirectory(644.nab_s)        # C
 if (TEST_SUITE_FORTRAN)
   add_subdirectory(649.fotonik3d_s) # Fortran
   add_subdirectory(654.roms_s)      # Fortran
 endif ()
 
 add_subdirectory(996.specrand_fs)  # C

External/SPEC/SpecCPU2017.cmake

if (NOT DEFINED BENCHMARK)
     add_definitions(-DSPEC -DSPEC_CPU -DNDEBUG)
 
     if (RATE)
       # rate benchmarks never use parallelism
       add_definitions(-DSPEC_AUTO_SUPPRESS_OPENMP)
     endif ()
 
     # Portability flags
+    # SPEC_AUTO_BYTEORDER_value variable is used for Fortran tests that use specpp
     if (ENDIAN STREQUAL "little")
       add_definitions(-DSPEC_AUTO_BYTEORDER=0x12345678)
+      set(SPEC_AUTO_BYTEORDER_VALUE 0x12345678)
     elseif (ENDIAN STREQUAL "big")
       add_definitions(-DSPEC_AUTO_BYTEORDER=0x87654321)
+      set(SPEC_AUTO_BTYEORDER_VALUE 0x87654321)
     endif ()
 
     check_type_size("long long" SIZEOF_LONG_LONG)
     check_type_size("long" SIZEOF_LONG)
     check_type_size("int" SIZEOF_INT)
+    # SPEC_PTR_TYPE variable is used for Fortran tests that use specpp
     if (CMAKE_SIZEOF_VOID_P EQUAL 4 AND SIZEOF_LONG_LONG EQUAL 8 AND SIZEOF_LONG EQUAL 4 AND SIZEOF_INT EQUAL 4)
       add_definitions(-DSPEC_ILP32)
+      set(SPEC_PTR_TYPE ILP32)
     elseif (CMAKE_SIZEOF_VOID_P EQUAL 8 AND SIZEOF_LONG_LONG EQUAL 8 AND SIZEOF_LONG EQUAL 4 AND SIZEOF_INT EQUAL 4)
       add_definitions(-DSPEC_P64)
+      set(SPEC_PTR_TYPE P64)
     elseif (CMAKE_SIZEOF_VOID_P EQUAL 8 AND SIZEOF_LONG_LONG EQUAL 8 AND SIZEOF_LONG EQUAL 8 AND SIZEOF_INT EQUAL 4)
       add_definitions(-DSPEC_LP64)
+      set(SPEC_PTR_TYPE LP64)
     elseif (CMAKE_SIZEOF_VOID_P EQUAL 8 AND SIZEOF_LONG_LONG EQUAL 8 AND SIZEOF_LONG EQUAL 8 AND SIZEOF_INT EQUAL 8)
       add_definitions(-DSPEC_ILP64)
+      set(SPEC_PTR_TYPE ILP64)
     else ()
       message(FATAL_ERROR "SPEC CPU 2017 unsupported data model (supported: ILP32/LLP64/LP64/ILP64)")
     endif ()
 
     if (TARGET_OS STREQUAL "Linux")
       add_definitions(-DSPEC_LINUX) # 526.blender_r
     endif ()
 
     if(ARCH STREQUAL "x86" AND TARGET_OS STREQUAL "Linux")
       if (CMAKE_SIZEOF_VOID_P EQUAL 8)
         # Linux x86_64
else ()
       message("CMAKE_SIZEOF_VOID_P: ${CMAKE_SIZEOF_VOID_P}")
       message(FATAL_ERROR
         "Don't know portability flags for SPEC CPU 2017 on this platform")
     endif ()
 
     # No OpenMP for the moment, even for the _s suites.
     add_definitions(-DSPEC_SUPPRESS_OPENMP)
 
+    # Used by cam4, pop2, and wrf tests for portability
+    # https://gcc.gnu.org/gcc-10/porting_to.html
+    check_fortran_compiler_flag("-fallow-argument-mismatch" SUPPORTS_FALLOW_ARGUMENT_MISMATCH)
+
   endif ()
 endmacro()
 
 
 # Reuse the CMakeLists.txt of another benchmark.
 macro(speccpu2017_inherit _origin_path)
   include("${_origin_path}/CMakeLists.txt")
 endmacro ()

 macro(speccpu2017_prepare_rundir)
   foreach (_runtype IN LISTS TEST_SUITE_RUN_TYPE)
     if (EXISTS "${INPUT_all_DIR}")
       llvm_copy_dir(${PROG} "${RUN_${_runtype}_DIR}" "${INPUT_all_DIR}")
     endif ()
     llvm_copy_dir(${PROG} "${RUN_${_runtype}_DIR}" "${INPUT_${_runtype}_DIR}")
   endforeach ()
 endmacro()
+
+# Run specpp on all Fortran files, but do not do it recursively.
+# https://www.spec.org/cpu2017/Docs/specpp.html
+# specpp is needed due to legacy use of filepp by climate/weather codes.
+macro(speccpu2017_run_specpp)
+  set(_specpp_bin ${TEST_SUITE_SPEC2017_ROOT}/bin/specpp)
+  # We don't use GLOB_RECURSE below because only the *.F90 files in the top
+  # level directory needs to be processed by specpp.
+  file(GLOB _specpp_sources ${SRC_DIR}/*.F90)
+  list(REMOVE_ITEM _specpp_sources ${SRC_DIR}/test.F90) # pop2 source file that doesn't get linked
+
+  # Add common specpp arguments used by all SPEC CPU 2017 tests.
+  # -U__FILE__ is included as a work around. Source files uses _FILE_
+  # as the filename without the path, but a few source files have a
+  # typo and use __FILE__ instead which is an absolute path. So specpp
+  # fails to do a replacement with just the filename (i.e. without the
+  # path as intended). This is usually a benign error, but __FILE__
+  # can lead to a deeply nested directory due CMake's build
+  # directories. As a result, the source file line length becomes
+  # incredibly long, so you end up with a ridiculously long line that
+  # some compilers, like GCC, are unable to compile.
+  set(ARG_COMMON -w -U__FILE__ -DSPEC -DSPEC_CPU -DNDEBUG -DSPEC_CASE_FLAG
+    -DSPEC_AUTO_BYTEORDER=${SPEC_AUTO_BYTEORDER_VALUE} -DSPEC_${SPEC_PTR_TYPE} -DSPEC_SUPPRESS_OPENMP -I${SRC_DIR})
+
+  foreach(_absfilename ${_specpp_sources})
+    get_filename_component(_filename ${_absfilename} NAME)
+    add_custom_command(
+      OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/${_filename}"
+      COMMAND "${_specpp_bin}" ${ARG_COMMON} ${ARGN} "${_absfilename}" > "${CMAKE_CURRENT_BINARY_DIR}/${_filename}"
+      DEPENDS "${_absfilename}"
+      VERBATIM
+      )
+    target_sources(${PROG} PRIVATE "${CMAKE_CURRENT_BINARY_DIR}/${_filename}")
+  endforeach ()
+endmacro()