[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

12

12–15

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 ()

35

[typo] note

37

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.

66

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?

83–84

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.

105

If it works without IGNORE_WHITESPACE, is it even needed?

109

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 ...)?

437

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

458

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

12–15

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

66

Correct

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

83–84

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.

105

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

We can take it out.

109

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.

437

Good idea.

458

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

105

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

109

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.