[PATCH] D142441: [Docs] Add best practices for regression tests
Sanjay Patel via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 24 06:32:12 PST 2023
spatel added a comment.
LGTM - see inline for minor edits.
Nice! I was wondering if we could adapt/copy:
https://developers.redhat.com/articles/2022/12/20/how-contribute-llvm#
...to the LLVM doc pages directly. This is a great start.
================
Comment at: llvm/docs/TestingGuide.rst:321-325
+- Unless the test is for a crash or assertion failure, first commit the test
+ with baseline check lines, i.e. either showing a miscompile or missing
+ optimization. Such test additions can be committed without review. Your
+ actual patch will then only contain a check line diff, which makes it easier
+ to see the effect of your patch.
----------------
```
- If the test does not crash, assert, or infinite loop, commit the test
with baseline check-lines. 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
(link to getting started page?).
```
================
Comment at: llvm/docs/TestingGuide.rst:328-329
+ section) whenever feasible.
+- Especially for regression tests, include a comment on what is being tested,
+ and include a reference to any relevant issue on the bug tracker.
+- Avoid undefined behavior unless necessary. For example, do not use patterns
----------------
No need to repeat clause about regression tests - this section only applies to those.
```
- 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").
```
================
Comment at: llvm/docs/TestingGuide.rst:330-332
+- Avoid undefined behavior unless necessary. For example, do not use patterns
+ like ``br i1 undef``, which are likely to break as a result of future
+ optimizations.
----------------
```
- Avoid undefined behavior or values unless necessary.
```
================
Comment at: llvm/docs/TestingGuide.rst:338-339
+ running the test through ``opt -S -passes=instnamer``.
+- Try to give SSA variables meaningful names, and avoid retaining complex names
+ generated by the optimization pipeline (such as ``%foo.0.0.0.0.0.0``).
+
----------------
Remove "SSA" - this is true in general for source-level tests too?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D142441/new/
https://reviews.llvm.org/D142441
More information about the llvm-commits
mailing list