Add update_any_test_checks.py convenience utility

[nhaehnle]

Summary

Given a list of test files, this utility will run (optionally in
parallel) the corresponding update_*_test_checks tool for all given
tests that have automatically generated assertions.

[foad]

Great idea.

I always run update_llc_test_checks.py with --llc-binary=... and update_test_checks.py with --opt-binary=... How does this new tool support that? Or do I need to change my workflow?

Also, probably out of scope for this patch, but does it handle files that have checks generated by two or more tools, like test/CodeGen/AMDGPU/infinite-loop.ll?

[mtrofin]

Nice!

could you run yapf or some other formatter, to avoid long lines (easier diffing in the future). Maybe leave a comment as to how to run yapf (or whatever tool), too?

(Hmm... thinking out loud, I think maybe we can add a yapf.cfg or something like that in the root of the repo, just like we have .clang-format. Anyway, not for this patch)

llvm/utils/update_any_test_checks.py

72

consider maybe also saying which file this unknown tool was found into?

[nhaehnle]

Thank you for the quick review! I will give yapf a try.

I always run update_llc_test_checks.py with --llc-binary=... and update_test_checks.py with --opt-binary=... How does this new tool support that? Or do I need to change my workflow?

It doesn't support it at the moment. In my local setup, I source an environment that has all the tools in its PATH. I find that more convenient for other reasons, so didn't try to figure out how to make the alternative work.

Also, probably out of scope for this patch, but does it handle files that have checks generated by two or more tools, like test/CodeGen/AMDGPU/infinite-loop.ll?

Huh, interesting. I think what we'd want to do is to run all the UTC tools in sequence, from the same executor submit. I played around with this just now, and it looks like the tools don't reliably update the header notes about autogenerated tests. So I'd keep that as a separate change if desired.

llvm/utils/update_any_test_checks.py

72

Good idea.

[arsenm]

I still think having different update_test*_* scripts was a mistake and we should have had one script that understands multiple forms of output. The current situation is divergent behavior with multiple run lines. Currently one of update_test_checks, update_mir_test_checks, and update_llc_test_checks understands the other ones run lines, but the other doesn't. They also can mess up each others superfluous whitespace and "." checks

[arichardson]

In D139100#3963822, @nhaehnle wrote:

Thank you for the quick review! I will give yapf a try.

I always run update_llc_test_checks.py with --llc-binary=... and update_test_checks.py with --opt-binary=... How does this new tool support that? Or do I need to change my workflow?

It doesn't support it at the moment. In my local setup, I source an environment that has all the tools in its PATH. I find that more convenient for other reasons, so didn't try to figure out how to make the alternative work.

Also, probably out of scope for this patch, but does it handle files that have checks generated by two or more tools, like test/CodeGen/AMDGPU/infinite-loop.ll?

Huh, interesting. I think what we'd want to do is to run all the UTC tools in sequence, from the same executor submit. I played around with this just now, and it looks like the tools don't reliably update the header notes about autogenerated tests. So I'd keep that as a separate change if desired.

I think one of the problems is that tools refuse to update if there is no comment/a comment from another tool unless you pass the --force-update flag to ignore that.

IMO it would be nice if we had a single script to update everything (at least for _test_checks/llc_test_checks/cc_test_checks, not sure about the analyzer/mir ones since this scripts share less code).

In D139100#3963298, @foad wrote:

Great idea.

I always run update_llc_test_checks.py with --llc-binary=... and update_test_checks.py with --opt-binary=... How does this new tool support that? Or do I need to change my workflow?

One option would be to forward those arguments - maybe all tools should just accept those flags and ignore them if the tool is not used (or --llvm-bin). Another alternative would be to generate a script inside the build dir and then you can run <buil-dir>/utils/update_any_test_checks <path to dir>. I did something like that for the crash testcase reduction tool in CHERI LLVM: https://github.com/CTSRD-CHERI/llvm-project/blob/master/clang/utils/creduce_crash_testcase.py.in

[jdoerfert]

Thanks!

llvm/utils/update_any_test_checks.py

+#!/usr/bin/env python3
+
+"""Dispatch to update_*_test_checks.py scripts automatically in bulk
+
+Given a list of test files, this script will invoke the correct
+update_test_checks-style script, skipping any tests which have not previously
+had assertions autogenerated.
+"""
+
+from __future__ import print_function
+
+import argparse
+import os
+import re
+import subprocess
+import sys
+from concurrent.futures import ThreadPoolExecutor
+
+RE_ASSERTIONS = re.compile(
+    r'NOTE: Assertions have been autogenerated by ([^\s]+)( UTC_ARGS:.*)?$')
+
+def find_utc_tool(search_path, utc_name):
+  """
+  Return the path to the given UTC tool in the search path, or None if not
+  found.
+  """
+  for path in search_path:
+    candidate = os.path.join(path, utc_name)
+    if os.path.isfile(candidate):
+      return candidate
+  return None
+
+def run_utc_tool(utc_name, utc_tool, testname):
+  result = subprocess.run([utc_tool, testname], stdout=subprocess.PIPE,
+                          stderr=subprocess.PIPE)
+  return (result.returncode, result.stdout, result.stderr)
+
+def main():
+  from argparse import RawTextHelpFormatter
+  parser = argparse.ArgumentParser(description=__doc__,
+                                   formatter_class=RawTextHelpFormatter)
+  parser.add_argument(
+      '--jobs', '-j', default=1, type=int,
+      help='Run the given number of jobs in parallel')
+  parser.add_argument(
+      '--utc-dir', nargs='*',
+      help='Additional directories to scan for update_*_test_checks scripts')
+  parser.add_argument('tests', nargs='+')
+  config = parser.parse_args()
+
+  script_name = os.path.basename(__file__)
+  if config.utc_dir:
+    utc_search_path = config.utc_dir[:]
+  else:
+    utc_search_path = []
+  utc_search_path.append(os.path.join(os.path.dirname(script_name),
+                                      os.path.pardir))
+
+  not_autogenerated = []
+  utc_tools = {}
+  have_error = False
+
+  with ThreadPoolExecutor(max_workers=config.jobs) as executor:
+    jobs = []
+
+    for testname in config.tests:
+      with open(testname, 'r') as f:
+        header = f.readline().strip()
+        m = RE_ASSERTIONS.search(header)
+        if m is None:
+          not_autogenerated.append(testname)
+          continue
+
+        utc_name = m.group(1)
+        if utc_name not in utc_tools:
+          utc_tools[utc_name] = find_utc_tool(utc_search_path, utc_name)
+          if not utc_tools[utc_name]:
+            print(f"{utc_name}: not found (used in {testname})",
+                  file=sys.stderr)
+            have_error = True
+            continue
+
+        future = executor.submit(run_utc_tool, utc_name, utc_tools[utc_name],
+                                 testname)
+        jobs.append((testname, future))
+
+    for testname, future in jobs:
+      return_code, stdout, stderr = future.result()
+
+      print(f"Update {testname}")
+      stdout = stdout.decode(errors='replace')
+      if stdout:
+        print(stdout, end='')
+        if not stdout.endswith('\n'):
+          print()
+
+      stderr = stderr.decode(errors='replace')
+      if stderr:
+        print(stderr, end='')
+        if not stderr.endswith('\n'):
+          print()
+      if return_code != 0:
+        print(f"Return code: {return_code}")
+        have_error = True
+
+  if have_error:
+    sys.exit(1)
+
+  if not_autogenerated:
+    print("Tests without autogenerated assertions:")
+    for testname in not_autogenerated:
+      print(f"  {testname}")
+
+if __name__ == '__main__':
+  main()