• More explicit comment about the purpose and usage of driver_run script.

In D104150#2826484, @rovka wrote:

In D104150#2824793, @naromero77 wrote:

In D104150#2818673, @rovka wrote:

Is this expected to pass with current upstream flang?
I gave this a quick spin on an AArch64 machine similar to our buildbots (same hardware, not necessarily same ubuntu version, system compiler etc) and I'm seeing one failure in FM509:

/path/to/build/bin/flang -DNDEBUG  -w -Werror=date-time -c /path/to/llvm-test-suite/Fortran/UnitTests/fcvs21_f95/FM509.f -o CMakeFiles/FM509.dir/FM509.f.o
f18-c234.f90:237:12:

  237 |  CALL sn512(c1n001(5)(2:9), cvcomp)
      |            1
Error: Actual argument contains too few elements for dummy argument ‘c1d001’ (19/48) at (1)
execvp(gfortran) failed: 
flang: in /path/to/test-suite-sandbox/build/Fortran/UnitTests/fcvs21_f95, f18 failed with exit status 0: /path/to/build/bin/f18 -module-suffix .f18.mod -DNDEBUG -w -Werror=date-time -c /path/to/llvm-test-suite/Fortran/UnitTests/fcvs21_f95/FM509.f -o CMakeFiles/FM509.dir/FM509.f.o

I'm attaching the produced f18-c234.f90 in case anyone wants to have a look.

f18-c234.f909 KBDownload

Sorry for the slow reply. I am on vacation this week and will return to a regular schedule next week. I had no failures with an GCC 7.5, but did have one failure with GCC 10.2 with this same test. Are you using the Flang throw away driver? If so, which GCC are you using.

No worries :) I'm using GCC 9.3.0.

Can you try running it again with the latest version of the patch? It should compile and pass now. This test needs the -std=legacy flag.

It would be pretty easy to remove this test if we decided that is the best thing to do.

For now it would be best to investigate :)

If the test looks fishy (e.g. isn't conformant to the standard or uses obsolete features that we're not interested in), then of course we can remove it.

It kind of looks like this type of thing:
https://gcc.gnu.org/gcc-10/porting_to.html

• -std=legacy flag needed by newer versions of GCC.

In D104150#2825117, @Meinersbur wrote:

In D104150#2814958, @naromero77 wrote:

In D104150#2814417, @Meinersbur wrote:

Could you delete unused driver_parse and driver_run?

However, if anyone ever needed to update the reference results, the driver_run script would come in handy.

Does driver_run come from the NIST Validation suite? Generally, we would not to support multiple means to run tests (llvm-lit and driver_run).

If its only purpose is to update reference_output files, it should make that clear in comments.
There is the possibility to extend llvm-lit to update all reference-output (instead of comparing to it) without each benchmark having its own script.

In D104150#2818673, @rovka wrote:

Is this expected to pass with current upstream flang?
I gave this a quick spin on an AArch64 machine similar to our buildbots (same hardware, not necessarily same ubuntu version, system compiler etc) and I'm seeing one failure in FM509:

/path/to/build/bin/flang -DNDEBUG  -w -Werror=date-time -c /path/to/llvm-test-suite/Fortran/UnitTests/fcvs21_f95/FM509.f -o CMakeFiles/FM509.dir/FM509.f.o
f18-c234.f90:237:12:

  237 |  CALL sn512(c1n001(5)(2:9), cvcomp)
      |            1
Error: Actual argument contains too few elements for dummy argument ‘c1d001’ (19/48) at (1)
execvp(gfortran) failed: 
flang: in /path/to/test-suite-sandbox/build/Fortran/UnitTests/fcvs21_f95, f18 failed with exit status 0: /path/to/build/bin/f18 -module-suffix .f18.mod -DNDEBUG -w -Werror=date-time -c /path/to/llvm-test-suite/Fortran/UnitTests/fcvs21_f95/FM509.f -o CMakeFiles/FM509.dir/FM509.f.o

I'm attaching the produced f18-c234.f90 in case anyone wants to have a look.

f18-c234.f909 KBDownload

• More robust way to get rid of fort.* files.
• More compact code that achieves the same result.
• Remove driver_parse script.

In D104150#2814417, @Meinersbur wrote:

Could you delete unused driver_parse and driver_run?

• Move hello test to its own subdirectory on UnitTests
• include Fortran helper moduler in top-level CMakeLists.txt to permit selection of individual UnitTests.
• Additional CMakeLists.txt needed at this intermediate level.
• Update comment in Fortran subdirectory CMakeLists.

• Fix typos and other grammatical issues.

• Remove extraneous tab characters which messed up indentation.
• Move comments before macro definition and update description.
• CMAKE_CURRENT_BINARY_DIR did not need to be added as an included directory.
• Adjust name of target object library to lowercase and append BENCHMARK_SUITE_TYPE

• Remove SPECPP from variable names.
• Improve description of speccpu2017_run_specpp

• 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.

• big-endian flag fix for GCC and Intel.

Was still missing earlier commits.

Trying to send commits again.

• 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

LGTM

Confirming that all tests are passing with clang/clang++.

Ran the entire test suite with GCC 10.2.0. For the record, there was one failure:

Trying again to send the patch.

• Use standard convention for feature-test variables.
• Only add CMake Utility Modules that are actually used.

LGTM

Is there any negative impact to macOS X?

• Remove extraneous space.

• Use plain vanilla diff instead of specdiff.
• Use target_compile_definitions instead.
• Fix typo and clarify comment.
• Remove redundant explicit dependencies.

• Get rid of redudant warning in top-level CMakeLists when Ninja is not used.
• Improve line wrapping.

• Improved warning message for CMAKE_GENERATOR.

In D96746#2566974, @Meinersbur wrote:

LLVM required cmake 3.13.4, I think we can bump the version for llvm-test-suite as well.

• Run ROMS test out of run directory.
• Revert "Helper macro for managing intermediate dependencies on Fortran modules."
• Add warning about CMAKE_GENERATORS that are not Ninja.
• Fix comment to reflect ROMS test.
• Intermediate target dependencies are not needed when using Ninja.

In 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.

In D96746#2566172, @Meinersbur wrote:

As far as I know, cmake detect module dependency itself, which would make speccpu2017_add_mod unnecessary.

It is not really clear from the CMake docs that it can figure out complex module dependencies without intermediary targets. Searching online, it looks like many Fortran-centric projects have come up with different solutions.