This is an archive of the discontinued LLVM Phabricator instance.

[UpdateTestChecks] Add update_analyze_test_checks.py for cost model analysis generation
ClosedPublic

Authored by RKSimon on Apr 4 2018, 10:54 AM.

Details

Summary

The script allows the auto-generation of checks for cost model tests to speed up their creation and help improve coverage, which will help a lot with PR36550.

I've branched this from update_test_checks.py - as can be seen, the diff is quite small but I wasn't sure if we wanted to have a script that tried to handle both analysis and ir in the same test files or not?

If the need arises we can add support for other analyze passes as well, but the cost models was the one I needed to get done - at the moment it just warns that its unsupported.

I've regenerated a couple of x86 test files to show the effect.

Diff Detail

Repository
rL LLVM

Event Timeline

RKSimon created this revision.Apr 4 2018, 10:54 AM
MaskRay added a comment.EditedApr 4 2018, 11:24 AM

Shall we use #!/usr/bin/env python3 for newer utility scripts?

I know https://llvm.org/docs/GettingStarted.html#software specifies >=2.7 (presumably Python 2) and some infrastructure rely on Python 2. But these utility scripts are independent and can be migrated to Python 3. https://pythonclock.org/

utils/UpdateTestChecks/*.py are Python 2/3 compatible but utils/update_{,llc_}test_checks.py uses python2.7 because of reviewers' stickness (when I refactored the code) with Python 2.

MaskRay added inline comments.Apr 4 2018, 11:27 AM
utils/UpdateTestChecks/common.py
168

There is also a similar function add_asm_checks in utils/UpdateTestChecks/asm.py

Someone should unify it with this function

Shall we use #!/usr/bin/env python3 for newer utility scripts?

I know https://llvm.org/docs/GettingStarted.html#software specifies >=2.7 (presumably Python 2) and some infrastructure rely on Python 2. But these utility scripts are independent and can be migrated to Python 3. https://pythonclock.org/

utils/UpdateTestChecks/*.py are Python 2/3 compatible but utils/update_{,llc_}test_checks.py uses python2.7 because of reviewers' stickness (when I refactored the code) with Python 2.

I have no preference either way for this - naturally this would definitely mean that it isn't merged into update_test_checks.py

RKSimon updated this revision to Diff 141125.Apr 5 2018, 4:02 AM

Updated now that common.add_checks has been committed (and add_asm_checks converted to a wrapper)

RKSimon updated this revision to Diff 141130.Apr 5 2018, 5:13 AM

Bumped to python3 as its a new script

gbedwell added inline comments.Apr 5 2018, 5:25 AM
utils/update_analyze_test_checks.py
69

This line won't work in Python 3. If you want to make it compatible with both you'd probably be best off replacing with a call to sys.stderr.write (and add an explicit newline). Same goes for the other print statements in the file.

RKSimon updated this revision to Diff 141144.Apr 5 2018, 6:17 AM

Moved back to python2.7

RKSimon updated this revision to Diff 141311.Apr 6 2018, 4:32 AM

Don't bother with regex ir variables, make the analysis check strings as easy as possible to read

andreadb accepted this revision.Apr 6 2018, 4:41 AM

I like this version of the script. The output is easier to read now that you removed all the needless tablegen variables with regexpr.
I don't have a strong opinion on which version to use for this script (whether python 2.7 or 3).
So, the script looks good to me.

This revision is now accepted and ready to land.Apr 6 2018, 4:41 AM

Yeah. LGTM too. In general we should probably put some effort into refactoring the scripts so that they work with both Python versions but it seems when the default was discussed a couple of months back ( http://lists.llvm.org/pipermail/llvm-dev/2018-February/thread.html#120907 ) there were still too many OSs in use shipping with only 2.7 by default, so it makes sense to leave this in line with the existing scripts until there can be a coordinated effort to switch.

This revision was automatically updated to reflect the committed changes.