This is an archive of the discontinued LLVM Phabricator instance.

[llvm-test-suite][Fortran/gfortran] Refactor build system
ClosedPublic

Authored by tarunprabhu on Jul 3 2023, 10:27 AM.

Details

Summary

This is #2 in the effort to enable the "compile" tests in the gfortran test suite.

This refactors the build system by removing some redundant code from the regression and torture directories and consolidating most of the build system logic into gfortran/CMakeLists.txt. This simplifies the build scripts in the directories containing the actual tests which ought to make them easier to maintain, especially if they are updated to pull in any new tests added to upstream gfortran.

There is also some cleanup including more consistent capitalization of variable names and more inline comments describing some of the peculiarities of the approach.

Diff Detail

Repository
rT test-suite

Event Timeline

tarunprabhu created this revision.Jul 3 2023, 10:27 AM
tarunprabhu requested review of this revision.Jul 3 2023, 10:27 AM

Thanks for the refactoring!
LGTM overall, but I'm hitting a compilation error on AArch64 (see comment).

Fortran/gfortran/CMakeLists.txt
129

I get an error on AArch64 when -mtune=amdfam10 is used. It seems to be an X86-specific flag.

241

nit: s/fail/file/

537–547

It seems there is some code for parsing DejaGNU's directives that is common between compile and execute tests.
As it's currently only a few lines, it doesn't seem worth to move it to a new function now, but it's something to consider in the future, if more common directives appear.

Fixed typos.

Added -mtune=amdfam10 to the set of blacklisted flags. This is a target-specific flag, and once the DejaGNU target directives are handled correctly, this can be removed (along with some other flags) from the blacklist.

tarunprabhu marked 2 inline comments as done.Jul 3 2023, 12:21 PM
tarunprabhu added inline comments.
Fortran/gfortran/CMakeLists.txt
129

I intend to work on enabling the target directives after I get the remaining "compile" tests operational. Handling those directives properly should reduce the likelihood of these sorts of problems.

241

I would argue that this is a bug, not merely a nit :-)

537–547

Yes, I agree,

The target directives are currently ignored and need to be handled (I intend to do so once I enable the compile-tests - if not before if it turns out to cause major problems). That would be a good time to address this.

luporl accepted this revision.Jul 3 2023, 12:33 PM
This revision is now accepted and ready to land.Jul 3 2023, 12:33 PM
tblah added a subscriber: tblah.Jul 4 2023, 10:10 AM
This revision was automatically updated to reflect the committed changes.
tarunprabhu marked 2 inline comments as done.