[PATCH] D142441: [Docs] Add best practices for regression tests

[Nikita Popov via Phabricator via llvm-commits]

nikic updated this revision to Diff 491793.
nikic added a comment.

Incorporate review feedback. I've split off a separate section for the precommit workflow, as it's longer than a single bullet point.


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

https://reviews.llvm.org/D142441

Files:
  llvm/docs/TestingGuide.rst


Index: llvm/docs/TestingGuide.rst
===================================================================
--- llvm/docs/TestingGuide.rst
+++ llvm/docs/TestingGuide.rst
@@ -315,6 +315,43 @@
   update_test_checks.py
   opt
 
+Precommit workflow for tests
+----------------------------
+
+If the test does not crash, assert, or infinite loop, commit the test with
+baseline check-lines first. That is, the test will show a miscompile or
+missing optimization. Add a "TODO" or "FIXME" comment to indicate that
+something is expected to change in a test.
+
+A follow-up patch with code changes to the compiler will then show check-line
+differences to the tests, so it is easier to see the effect of the patch.
+Remove TODO/FIXME comments added in the previous step if a problem is solved.
+
+Baseline tests (no-functional-change or NFC patch) may be pushed to main
+without pre-commit review if you have commit access.
+
+Best practices for regression tests
+-----------------------------------
+
+- Use auto-generated check lines (produced by the scripts mentioned above)
+  whenever feasible.
+- Include comments about what is tested/expected in a particular test. If there
+  are relevant issues in the bug tracker, add references to those bug reports
+  (for example, "See PR999 for more details").
+- Avoid undefined behavior and poison/undef values unless necessary. For
+  example, do not use patterns like ``br i1 undef``, which are likely to break
+  as a result of future optimizations.
+- Minimize tests by removing unnecessary instructions, metadata, attributes,
+  etc. Tools like ``llvm-reduce`` can help automate this.
+- Outside PhaseOrdering tests, only run a minimal set of passes. For example,
+  prefer ``opt -S -passes=instcombine`` over ``opt -S -O3``.
+- Avoid unnamed instructions/blocks (such as ``%0`` or ``1:``), because they may
+  require renumbering on future test modifications. These can be removed by
+  running the test through ``opt -S -passes=instnamer``.
+- Try to give values (including variables, blocks and functions) meaningful
+  names, and avoid retaining complex names generated by the optimization
+  pipeline (such as ``%foo.0.0.0.0.0.0``).
+
 Extra files
 -----------
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D142441.491793.patch
Type: text/x-patch
Size: 2209 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230124/b1a392dc/attachment.bin>