This is an archive of the discontinued LLVM Phabricator instance.

[X86] Help update_llc_test_checks.py to recognise retl/retq to reduce CHECK duplication (PR35003)
ClosedPublic

Authored by RKSimon on May 29 2018, 9:54 AM.

Details

Summary

This patch replaces the --x86_extra_scrub command line argument to automatically support a second level of regex-scrubbing if it improves the matching of nearly-identical code patterns. The argument is still there to force extra matching if required.

This is mostly useful to help us share 32-bit/64-bit x86 vector tests which only differs by retl/retq instructions, but any scrubber can now technically support this, meaning test checks don't have to be needlessly obfuscated.

I've updated some of the existing checks that had been manually run with --x86_extra_scrub, to demonstrate the extra "ret{{[l|q]}}" scrub now only happens when useful, and re-run the sse42-intrinsics file to show extra matches - most sse/avx intrinsics files should be able to now share 32/64 checks.

Tested with the opt/analysis scripts as well which share common code - AFAICT the other update scripts use their own versions.

Diff Detail

Repository
rL LLVM

Event Timeline

RKSimon created this revision.May 29 2018, 9:54 AM
gbedwell added inline comments.May 29 2018, 10:03 AM
utils/UpdateTestChecks/common.py
95 ↗(On Diff #148931)

This isn't the most descriptive field name. Can you either rename to something more descriptive or add a comment to explain the nature of the info?

106 ↗(On Diff #148931)

raw Bool arguments aren't pretty. Please at least make it a named argument, e.g.

do_scrub(body, scrubber, scrubber_args, extra=False)
RKSimon updated this revision to Diff 148939.May 29 2018, 10:24 AM

Updated field/argument names for clarity

andreadb accepted this revision.Jun 1 2018, 3:47 AM
andreadb added a subscriber: andreadb.

LGTM.
Thanks!

This revision is now accepted and ready to land.Jun 1 2018, 3:47 AM
gbedwell accepted this revision.Jun 1 2018, 6:13 AM

LGTM too :)

This revision was automatically updated to reflect the committed changes.