This is an archive of the discontinued LLVM Phabricator instance.

[Docs] Document scripts that are use to generate assertion in test cases
ClosedPublic

Authored by xgupta on Nov 1 2021, 9:11 AM.

Details

Summary

This patch document llvm/utils/update_* python scripts that are used to generate
assertions in many of the LLVM regression test cases.

Diff Detail

Event Timeline

xgupta created this revision.Nov 1 2021, 9:11 AM
xgupta requested review of this revision.Nov 1 2021, 9:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 1 2021, 9:11 AM
xgupta updated this revision to Diff 383817.Nov 1 2021, 9:12 AM

correct heading

We have a number of similar update scripts (which you allude to), please can you list them with a basic description of their purposes:

update_analyze_test_checks.py
opt --analyze --costmodel

update_cc_test_checks.py
clang c/c++

update_llc_test_checks.py
llc

update_mca_test_checks.py
llvm-mca

update_mir_test_checks.py
llc mir checks

update_test_checks.py
opt

xgupta added a comment.Nov 2 2021, 5:59 AM
This comment was removed by xgupta.
xgupta updated this revision to Diff 384054.Nov 2 2021, 6:11 AM

mention similar update script

xgupta retitled this revision from [Docs] Document update_llc_test_checks.py in TestingGuide.rst to [Docs] Document scripts that are use to generate assertion in test cases.Nov 2 2021, 6:17 AM
xgupta edited the summary of this revision. (Show Details)
MaskRay added inline comments.Nov 2 2021, 12:48 PM
llvm/docs/TestingGuide.rst
290

Probably only run on existing tests which have the notice line.

Change build to something else to indicate it is a placeholder. Many people don't use build as the build directory.

294

This may render poorly in rst. Place them into a code-block

Suggested by @jrtc27

I didn't actually suggest doing this, just gave you example commands to run. I kind of take the view that everyone should be capable of reading the first line in the file marked NOTE: and working out how to run the script in question, it has a full --help output.

llvm/docs/TestingGuide.rst
290

Probably only run on existing tests which have the notice line.

That's what the -u is for, will check the NOTE: line exists and matches the script name

jrtc27 added inline comments.Nov 2 2021, 12:58 PM
llvm/docs/TestingGuide.rst
285
290

Which means, if you're documenting things here, you should probably note what -u does, otherwise people writing new test files who come to run update_llc_test_checks.py -u might get rather confused as to why it's doing nothing...

298

C/C++, or clang/clang++

301

perhaps to distinguish from MIR, I know people (myself included) can get confused about the difference between update_llc and update_mir, but it's about the output being generated, not the input

307
RKSimon added inline comments.Nov 2 2021, 1:19 PM
llvm/docs/TestingGuide.rst
290

Its probably better to give an example that just specifies a couple of test files (e.g. 'test1.ll .... test3.ll' or something), trying to run the script on every file is slow and usually breaks somewhere that is then tricky to isolate.

292

I meant expand the brief descriptions that I put in the comment, but assuming this renders properly I guess its OK.

xgupta updated this revision to Diff 384300.Nov 2 2021, 7:00 PM

address comments

xgupta edited the summary of this revision. (Show Details)Nov 2 2021, 7:04 PM

Thanks for the review.

llvm/docs/TestingGuide.rst
290

Change build to something else to indicate it is a placeholder. Many people don't use build as the build directory.

I have seen LLVM getting started and typofix tutorial, most llvm documents use build as standard, I also think it is better for copy-paste.

292

Probably do it some other day after using them a bit.

294

Thanks, I tested the generated .html file, it looks fine now.

LGTM (with one minor) - although I don't have a sphinx build around to check the formatting works.

Any other comments?

llvm/docs/TestingGuide.rst
302

C/C++, or clang/clang++ (IR checks)

xgupta added a comment.Nov 3 2021, 3:06 AM

I have attached the screenshot.

RKSimon added inline comments.Nov 3 2021, 3:33 AM
llvm/docs/TestingGuide.rst
294

"These are the most common scripts and their purposes/applications in generating assertions:"

MaskRay accepted this revision.Nov 3 2021, 9:36 AM

Thanks!

llvm/docs/TestingGuide.rst
291
292
This revision is now accepted and ready to land.Nov 3 2021, 9:36 AM
This revision was landed with ongoing or failed builds.Nov 3 2021, 9:54 AM
This revision was automatically updated to reflect the committed changes.
jrtc27 added inline comments.Nov 3 2021, 10:44 AM
llvm/docs/TestingGuide.rst
298

I meant "change it to clang C/C++ or change it to clang/clang++", not "change it to a literal C/C++, or clang/clang++"

xgupta added inline comments.Nov 3 2021, 11:05 AM
llvm/docs/TestingGuide.rst
298

Maybe then I would have to say contributions are welcome to further improve/correct it.