[PATCH] D96746: [test-suite] SPEC2017 CPU ROMS floating point tests.

Nichols A. Romero via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 16 14:31:31 PST 2021


naromero77 added a comment.

In D96746#2566172 <https://reviews.llvm.org/D96746#2566172>, @Meinersbur wrote:

> As far as I know, cmake detect module dependency itself, which would make `speccpu2017_add_mod` unnecessary.
>
> I applied the patch locally and change the compilation part to just `speccpu2017_add_executable()` and it still compiles.





================
Comment at: External/SPEC/CFP2017rate/554.roms_r/CMakeLists.txt:369
+# dependency on executable, will include transitive dependencies as well
+add_dependencies(${PROG} ${PROG}_Fortran_MOD_4)
+
----------------
Meinersbur wrote:
> Consider using target_link_libraries (https://cmake.org/cmake/help/latest/command/target_link_libraries.html#linking-object-libraries) for linking object libraries.
> 
> The tutorial uses generator expressions (`$<TARGET_OBJECTS:${PROG}_Fortran_MOD_4>`) which are missing here?
> Consider using target_link_libraries (https://cmake.org/cmake/help/latest/command/target_link_libraries.html#linking-object-libraries) for linking object libraries.
>
The downside is that we would have to bump the minimum CMake requirement to 3.12 and right now the minimum requirement is 3.4.3. I am concerned that it could break any buildbots out there using older CMake versions.

> The tutorial uses generator expressions (`$<TARGET_OBJECTS:${PROG}_Fortran_MOD_4>`) which are missing here?
I think you are right. I should also change it for the other `add_dependencies(..)` statements.



Repository:
  rT test-suite

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96746/new/

https://reviews.llvm.org/D96746



More information about the llvm-commits mailing list