[PATCH] D51465: Revamp test-suite documentation

[Kristof Beyls via Phabricator via llvm-commits]

kristof.beyls accepted this revision.
kristof.beyls added a comment.

I am as excited by this as the other reviewers.
Thank you very much for this, Matthias!

I only have a few nitpicks, see comments inline. Feel free to ignore my nitpicks - they're mostly personal opinion on my side, not essential.

Thanks!

Kristof



================
Comment at: docs/TestSuiteGuide.rst:19-20
+
+        % svn co http://llvm.org/svn/llvm-project/llvm/trunk llvm
+        % sudo pip install llvm/utils/lit
+        # You can also install the tool manually if lit is not available:
----------------
I tend to prefer documentation that shows how to set up a virtualenv, so that copy pasting commands from the documentation doesn't change your system set up by default.
However, in this case, I can also see the argument to not do that and keep documentation more concise.

I'm happy either way - just thought I'd share my thought.


================
Comment at: docs/TestSuiteGuide.rst:33
+
+#. Create a build directory and Use CMake to configure the suite. The
+   ``CMAKE_C_COMPILER`` option can be used to test a custom clang build.
----------------
s/Use/use/


================
Comment at: docs/TestSuiteGuide.rst:134
+unsuitable for performance measurements. Enabling the
+`TEST_SUITE_BENCHMARKING_ONLY` option will disable them.
+
----------------
s/option/cmake option/ ?


================
Comment at: docs/TestSuiteGuide.rst:288
+        % test-suite/utils/compare.py base0.json base1.json base2.json vs exp0.json exp1.json exp2.json
+
+
----------------
Maybe a pointer to LNT might be worthwhile here as a system that knows how to drive running the test-suite, collecting results, storing in a database, and analyzing it further?


================
Comment at: docs/TestSuiteGuide.rst:402-407
+.. NOTE::
+    The test-suite comes with a set of Makefiles that are considered deprecated.
+    They do not support newer testing modes like `Bitcode` or `Microbenchmarks`
+    and are harder to use.
+
+The old documentation can be found in the :doc:`TestSuiteMakefileGuide`.
----------------
I wonder if it wouldn't be better to have this note at the start.
I started thinking that when seeing that in docs/SourceLevelDebugging.rst in this patch, you changed a pointer to this guide, which explains the cmake/lit system, and then on the next line in docs/SourceLevelDebugging.rst, there is a command line with make, using the deprecated system.
Maybe it'll be less frustrating for people coming through that link to be notified immediately that there is an old, deprecated system based on make?


Repository:
  rL LLVM

https://reviews.llvm.org/D51465