Add gfortran tests to llvm-test-suite

Subprojects Flang
1

I have been working on adding the gfortran tests to LLVM’s test suite. I have largely followed the model used to add GCC’s C torture tests to the test suite.

There is a patch here (D146485) that adds the CMake configuration and license files to the repository. That is the primary patch that needs review. At the end of the post are links to other patches that add the test files themselves to the repository. All the patches will be committed together.

There are two broad categories of tests in the suite - “compile” tests and “execute” tests. The “compile” tests essentially test the compiler’s ability to print appropriate error messages, interact well the gdb, generate appropriate profiling information etc. The execute tests check the runtime behavior of the compiled code.

Currently, only the “execute” tests are built and run.

Of these, a number of tests have been disabled because they cannot be built. Some fail to build because they use features not yet implemented in flang and error message says as much. Others may also be a result of unimplemented features but that is not apparent from the error message. Some may be failing because they use unsupported (non-standard) extensions.

Q. Should these tests be left disabled in the test suite? In that case, the suite will build, but the list of disabled tests will have to be revisited periodically when the features that are causing their failure are implemented.

Of the tests that build, many fail to pass. I have not checked the reason for the failures. It may be that they are spurious, but they could also be legitimate failures.

Q. Should the currently failing tests also be disabled?

It may be useful to also enable the “compile” tests. Naturally, they expect GCC-specific error messages. As a temporary measure, we could just mark the relevant tests as “expected to fail” and we would at least have a set of “pass/fail” tests.

Q. From a cursory glance at the LLVM test-suite, it doesn’t seem to be geared to operate in this way i.e. it seems to be primarily intended to test the runtime performance/correctness of the compiler. If this is not the case, I could use help from someone more familiar with the test-suite to enable the compile tests at least in pass/fail mode.

Longer term, it may be useful to actually check for the appropriate error messages. This would involve replacing the DejaGNU annotations in the tests with LLVM-specific ones.

The patches that add the actual test files into the repo are here:

  1. D146475
  2. D146477
  3. D146478
  4. D146479
  5. D146480
  6. D146481

[EDIT]: Fixed links

2 Likes
3

I was responsible for doing this for the GCC C torture suite, and I’m glad to see us taking a similar approach for the gfortran test suite! I think it’s good to share test suites as much as possible, where they are testing issues that any toolchain may run into.

Looking at what I did for C, we have two separate lists:

  • UnsupportedTests which either rely on GCC-specific extensions that we have not implemented, or rely on undefined behaviour, or our CMake configuration doesn’t support them correctly (due to not extracting arguments correctly)
  • FailingTests which are tests that should be supported, but are not by clang (which I almost certainly should have filed issues about, but have not).

I realise that clang, when I added these, had a different proportion of UnsupportedTests vs FailingTests than flang will have, but making this distinction is useful (even moreso if we had a good xfail system - maybe I’ll look at that again for these), and i think should make it clear which list you want to make smaller, and which one you have no intention of making smaller.

For C, we did not, because the test suite is really about compilation+execution. We did say we’d revisit this and never got around to it.

I think broadly the llvm_test_executable_no_test(target, file) call in gcc_torture_execute_test means “check compilation doesn’t fail”, and llvm_test_run(…) means “also try running this test (maybe with arguments)”, when generating the underlying lit script used during testing. Note we never called llvm_test_verify because we never had reference outputs for the GCC torture suite. cmake/modules/TestFile.cmake explains a bit more about what is going on with this, but not quite enough. There’s no function for adding an XFAIL: ... line to the lit script but it would not be hard to add, and it would also be useful for some of the FailingTests in the C torture suites.

We do not want to make modifications to the GNU-licensed files in llvm-test-suite, instead, we want to just exclude stuff that doesn’t work, or ignore annotations we don’t understand. I think it’s fine we don’t check error messages, to be honest, but I can see the advantage in “check we reject this program like the relevant gnu toolchain does”. Maybe this is something for later, and it’s best to start with just the execution tests which have a clearer path to pass/fail.

One of my hopes is that we could get the llvm-test-suite so it’s usable to test gcc/gfortran, and i wonder if it would be possible to do something similar for the dejagnu system in the gcc/gfortran repositories so those tests can be run with clang/flang eventually. This will require some cross-project coordination, but I think this kind of cross-testing is good, actually (which is why I added the C torture suite).

I’m going to do a review of the final patch in the series. When i did the C torture suite patches, I think the patch landing all the sources of the tests wasn’t reviewed, only the CMake was, which seems like a reasonable approach here too.

2 Likes
4

Thanks lenary, your comments were very helpful.

That’s a good idea. I might need some help from folks more familiar with Fortran. I don’t know precisely what is and is not standard behavior. I might be wrong about this, but I think there is some non-standard behavior that we do support. Is there more that we (eventually) want to support.

So, the test-suite is primarily intended to check the runtime behavior of the compiled executable? But there is a possibility of expanding it to also test just the compilation itself? Did I interpret your comments correctly?

That would be great! Thanks!

5

Yes, Yes, and Yes :slight_smile:

6

Thanks @tarunprabhu for taking up the work of adding the gfortran tests. Also thanks @lenary for the comments and review.

I think enabling the execution tests is fine for a first step. If the tests pass on all platforms then it can be added to one of the existing CI so that the passing cases continue to work and we know of any regression. Would @omjavaid and team have time and interest in adding this to one of the existing CI?

I think most of the non-standard extensions are documented in Fortran Extensions supported by Flang — The Flang Compiler. If the frontend developers are busy, may be community members who are part of the standards committee can help identifying.

7

In that case, I should change the patch to also exclude tests that currently build but fail at runtime. So the categories of tests would be:

  1. Unsupported: These tests fail to build because of unsupported features that will never (?) be supported

  2. Unimplemented: These tests fail to build because of currently unimplemented features. Those features will eventually be implemented.

  3. Fails to build: These tests should build but don’t for some reason that is not 2.

  4. Failing: These tests build but fail at runtime when they should pass.

  5. Passing: These tests build and pass at runtime.

8

LLVM test suite isn’t set up to differentiate between “compile failure” and “execute failure”, so all these categories are effectively the same.

9

When we landed the original gcc c test suite, the first version of the patch contained all the tests and the build files - but during review I was asked to split the patch so only the CMake and Licenses were part of the review, and then to land all the test sources in a follow-up commit. This was to save reviewer time, as the testcases don’t need code quality review per se.

I think it would be reasonable to do the same here, on the understanding that a reviewer approving the CMake/Licence changes means they are happy for the GPL’d test sources to be landed in a follow-up commit that doesn’t go through specific code review. The promise needed from you in this case is that the follow-up commit adds sources from the GPL’d repo exactly as they are in GCC’s repo, without any modification.

1 Like
10

That sounds reasonable.

I will abandon those revisions with a note pointing to this post. I’ll also update the description on the patch containing the CMake and license files promising not to change any of the test cases from the gfortran test suite.

I am in the process of updating the cmake/license files patch based on feedback that I have received here, on the patch itself and a related post.

1 Like
11

We had some discussion about this. The cause was that the Fortran standard does not precisely define how to format floating-point values, but we wanted to check the exact output from Flang. As a result gfortran would fail the tests. See: ⚙ D128261 [Fortran] Relax relative tolerance for FCVS tests

An alternative would have been to put these flang-specific end-to-end tests into the llvm-project repository, but which is also not the place for end-to-end tests (OpenMP does does it anyway). I think the conclusion was we wanted a flag for llvm-test-suite that either specifically tests Flang or the Fortran specification.

12

There is no discrepancy between Flang and the Fortran standard! The standard makes some formatted output, including list-directed and NAMELIST output somewhat implementation-dependent, and Flang uses that freedom to emit minimal digit sequences for those cases. That output, when read back in to a real variable of the same precision, will be bitwise identical with the original data. The problem in that patch is that a comparison tool was being used that reads the formatted output back in to a 64-bit floating-point binary representation, for which list-directed formatted output of 32-bit reals would not be sufficiently precise.