Add SPEC2017 CPU CAM4 floating point rate and speed test.
Details
Details
Diff Detail
Diff Detail
- Repository
- rT test-suite
- Build Status
Buildable 91499 Build 127166: arc lint + arc unit
Event Timeline
| External/SPEC/CFP2017rate/527.cam4_r/CMakeLists.txt | ||
|---|---|---|
| 28 | bin/specdiff is a shell script that executes specperl. It contains if [ ! -x "${_bin}/specperl" ]; then
echo "specperl is not in ${_bin}; has the benchmark tree been installed?"
exit 1
fiAre you sure it doesn't need to be installed? It it is too much effort to support a root dir that is not a SPEC installation, I would compromise on that. It would also mean that CPU2017 cannot be put into the repository's test-suite-externals directory anymore and expected to work on any platform. | |
| 48–52 | The same source used for multiple artifacts is supposed to work for C/C++ (e.g. often used to build a shared and static library). Is this a bug in CMake's Fortran support? Another thing you could try to build the validator in another CMakeLists.txt added by add_subdirectory. Different add_subdirectories have independent dependency analyses. An (object) library might still be preferable to not compile these files twice. | |
| 61 | [very nit] double space | |
| 62 | target_compile_definitions would be preferred. | |
| 71 | [typo] recompilde | |
| 577–578 | According to https://cmake.org/cmake/help/latest/command/add_library.html#object-libraries, $<TARGET_OBJECTS:netcdf> would be added lie a source file, to add_executable not as a library dependency. The manual also mentions target_link_libraries(${PROG} PRIVATE netcdf) which should be able to replace both of these lines. | |
| External/SPEC/CFP2017rate/527.cam4_r/CMakeLists.txt | ||
|---|---|---|
| 28 | You are right. It needs to be installed. I think that I have found a work around though. specdiff make sure that the CAM4 validator outputs says PASS: 3 points. against the reference. So, specdiff <file> is equivalent to diff -b <file1> <file2>, the -b just says ignore extra whitespace. diff is available with Linux and MacOS X. I am not sure about Windows. | |
| 48–52 | Ninja came to a hard stop because of multiple rules generating the same module file error. I spent a while searching online for a work around. In some CMake issues, it looks like it was a known "behavior" with CMake + Ninja specific to Fortran. In previous version of ninja, it was only a warning, but now its a hard error. It looked like it was still possible to override this behavior with a optional flag to ninja. However since they are they same source files it was just as easy to make it an (object) library and to avoid the duplicate compile. I did not try to build the validator in a separate CMakeLists.txt and perhaps indeed this would have solved my issue as well. | |
Comment Actions
- Use plain vanilla diff instead of specdiff.
- Use target_compile_definitions instead.
- Fix typo and clarify comment.
- Remove redundant explicit dependencies.
| External/SPEC/CFP2017rate/527.cam4_r/CMakeLists.txt | ||
|---|---|---|
| 48–52 | Here is the most recent one which is still open: Here is another one, where the original issue was allegedly fixed: Here is an older one though that suggests its a build specific problem: | |
| 60 | Oops, sorry I missed the last double space. Will fix. | |
bin/specdiff is a shell script that executes specperl. It contains
if [ ! -x "${_bin}/specperl" ]; then echo "specperl is not in ${_bin}; has the benchmark tree been installed?" exit 1 fiAre you sure it doesn't need to be installed?
It it is too much effort to support a root dir that is not a SPEC installation, I would compromise on that. It would also mean that CPU2017 cannot be put into the repository's test-suite-externals directory anymore and expected to work on any platform.