[PATCH] D42805: [utils] Refactor utils/update_{, llc_}test_checks.py to share more code

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In https://reviews.llvm.org/D42805#996342, @asb wrote:

> In https://reviews.llvm.org/D42805#996333, @dberlin wrote:
>
> > These were actually deliberately split apart in the past year, so not sure what makes sense here.
>
>
> Are you referring to https://reviews.llvm.org/rL305208? It seems to me that attempts to make update_test_checks also handle llc checks were abandoned, but this patch takes a different strategy: maintaining separate 'frontends' and separate logic where it makes sense, but adding common utility functions where possible.


Yep - I killed the duplicate functionality because people couldn't tell which script to use when updating llc tests (and the scripts produced slightly different output, so it was causing bogus diffs).

It's entirely my fault that I didn't do this refactoring myself because I don't know python well enough. So I fully support making these more maintainable by people who actually know what they're doing. :)

One thing that might make the usage clearer - change the script name from "update_test_checks.py" to "update_opt_test_checks.py"? We would then want to update every regression test file that has the old name as part of the 'ADVERT' string, but that should be a mechanical/scriptable change? Or better - use the new script on every test file with the old string to prove that nothing is broken in this refactoring.


Repository:
  rL LLVM

https://reviews.llvm.org/D42805