[flang-commits] [PATCH] D102877: [flang][docs] Initial documentation for the Fortran LLVM Test Suite.

[Diana Picus via Phabricator via flang-commits]

rovka accepted this revision.
rovka added a comment.
This revision is now accepted and ready to land.

Thanks for writing this! I left a lot of comments, but most of them are just typos / nitpicks, otherwise it looks good :)



================
Comment at: flang/docs/FortranLLVMTestSuite.md:9
+The [LLVM Test Suite](https://github.com/llvm/llvm-test-suite) is a
+seperate git repo from the main LLVM project. We recommend that
+first-time users read through [LLVM Test Suite
----------------
Typo: seperate -> sep**a**rate


================
Comment at: flang/docs/FortranLLVMTestSuite.md:12
+Guide](https://llvm.org/docs/TestSuiteGuide.html) which describes the
+organizational structure of the test suite and describes how to run it.
+
----------------
Nitpick: You can skip the second "describes".


================
Comment at: flang/docs/FortranLLVMTestSuite.md:14
+
+Although the Flang driver is unable to generate code at this time, we
+are neverthelesss incrementally adding Fortran tests into the LLVM
----------------
This looks like the kind of sentence that's likely to get outdated and confuse people when flang does become capable of generating code. Do we have a better place to put this info? I don't remember if there's any "Current status" document that's constantly updated for flang.


================
Comment at: flang/docs/FortranLLVMTestSuite.md:16
+are neverthelesss incrementally adding Fortran tests into the LLVM
+Test Suite. We are currently testing against GFortran while we make
+progress towards completing the new Flang driver with full
----------------
Do we have or plan to have a buildbot doing this?


================
Comment at: flang/docs/FortranLLVMTestSuite.md:20
+
+## Running the LLVM TestSuite with Fortran
+
----------------
Nitpick: I think the other LLVM docs tend to use "test-suite" rather than "TestSuite" or "Test Suite", would be nice to be consistent.


================
Comment at: flang/docs/FortranLLVMTestSuite.md:31
+At the moment, there is only a "hello world" Fortran test. A current
+short coming in the design of the test suite is that building the
+C/C++ tests is conflated with building and running the Fortran tests,
----------------
Nitpick: I think "shortcoming" is a single word.


================
Comment at: flang/docs/FortranLLVMTestSuite.md:34
+i.e. it is not possible to only build and run the Fortran tests with
+the the exception of the [External
+tests](https://llvm.org/docs/TestSuiteGuide.html#external-suites)
----------------
Typo: the the -> the
Also, this sentence needs a '.' at the end :)


================
Comment at: flang/docs/FortranLLVMTestSuite.md:42
+Fortran tests from [SPEC CPU 2017](https://www.spec.org/cpu2017/). We
+strong encourage the use of the CMake Ninja (1.10 or later) generator
+due to better support for Fortran module dependency detection. Some of
----------------
Typo: strong -> strongly


================
Comment at: flang/docs/FortranLLVMTestSuite.md:45
+the SPEC CPU 2017 Fortran tests, those that are derived from climate
+codes, requires support for little-endian/big-endian byte swapping
+capabilities which we automatically detect at CMake configuration
----------------
Typo: requires -> require


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102877