update_mir_test_checks: Better handling of common prefixes

[nhaehnle]

Summary

Support the pattern where a test file uses multiple prefixes per run line:
one prefix that is unique to the run line, and additional prefixes that are
common with other run lines.

Decide on a per-function basis which prefix(es) to emit, based on which run
lines have the same output.

Move the renaming of vregs earlier, so that we can compare the output as it
would actually be printed in check lines.

[arichardson]

This looks reasonable to me, would it be possible to add a test showing the new behaviour?

[nhaehnle]

In D126411#3538893, @arichardson wrote:

This looks reasonable to me, would it be possible to add a test showing the new behaviour?

Yeah, I was wondering about that as well. Do you have any advice on how to do that? I didn't find any pre-existing tests, and it seems somewhat tricky to set up given that it's subject to changes in the code generator.

[arichardson]

In D126411#3540256, @nhaehnle wrote:

In D126411#3538893, @arichardson wrote:

This looks reasonable to me, would it be possible to add a test showing the new behaviour?

Yeah, I was wondering about that as well. Do you have any advice on how to do that? I didn't find any pre-existing tests, and it seems somewhat tricky to set up given that it's subject to changes in the code generator.

There is one mir test: https://github.com/llvm/llvm-project/tree/main/llvm/test/tools/UpdateTestChecks/update_mir_test_checks
I agree these tests are rather awkward since they depend on the codegen, but as long as they are simple there shouldn't be too many changes.

[nhaehnle]

Add a simple test case

[arsenm]

Thanks for finally doing this. This has wasted countless hours and is long overdue

llvm/test/tools/UpdateTestChecks/update_mir_test_checks/Inputs/x86-multiple-prefixes.ll.expected

3

Using -stop-after=instruction-select without -global-isel only works by accident. Probably should pick a different stopping point

[aemerson]

llvm/test/tools/UpdateTestChecks/update_mir_test_checks/Inputs/x86-multiple-prefixes.ll.expected

3

Yeah, the right one is probably finalize-isel

[nhaehnle]

llvm/test/tools/UpdateTestChecks/update_mir_test_checks/Inputs/x86-multiple-prefixes.ll.expected

3

Yeah, the right one is probably finalize-isel

Thanks, I'm adjusting the test before I push.

llvm/test/tools/UpdateTestChecks/update_mir_test_checks/Inputs/x86-multiple-prefixes.ll

+; RUN: llc < %s -mtriple=x86_64-apple-darwin -stop-after=finalize-isel | FileCheck --check-prefixes=CHECK,NOAVX %s
+; RUN: llc < %s -mtriple=x86_64-apple-darwin -stop-after=finalize-isel -mattr=avx | FileCheck --check-prefixes=CHECK,AVX %s
+
+@x = common global float zeroinitializer, align 4
+@z = common global <4 x float> zeroinitializer, align 16
+
+define void @zero32() nounwind ssp {
+  store float zeroinitializer, ptr @x, align 4
+  ret void
+}
+
+define void @zero128() nounwind ssp {
+  store <4 x float> zeroinitializer, ptr @z, align 16
+  ret void
+}
+

llvm/test/tools/UpdateTestChecks/update_mir_test_checks/Inputs/x86-multiple-prefixes.ll.expected

+; NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-apple-darwin -stop-after=finalize-isel | FileCheck --check-prefixes=CHECK,NOAVX %s
+; RUN: llc < %s -mtriple=x86_64-apple-darwin -stop-after=finalize-isel -mattr=avx | FileCheck --check-prefixes=CHECK,AVX %s
+
+@x = common global float zeroinitializer, align 4
+@z = common global <4 x float> zeroinitializer, align 16
+
+define void @zero32() nounwind ssp {
+  ; CHECK-LABEL: name: zero32
+  ; CHECK: bb.0 (%ir-block.0):
+  ; CHECK-NEXT:   [[MOV64rm:%[0-9]+]]:gr64 = MOV64rm $rip, 1, $noreg, target-flags(x86-gotpcrel) @x, $noreg :: (load (s64) from got)
+  ; CHECK-NEXT:   MOV32mi killed [[MOV64rm]], 1, $noreg, 0, $noreg, 0 :: (store (s32) into @x)
+  ; CHECK-NEXT:   RET 0
+  store float zeroinitializer, ptr @x, align 4
+  ret void
+}
+
+define void @zero128() nounwind ssp {
+  ; NOAVX-LABEL: name: zero128
+  ; NOAVX: bb.0 (%ir-block.0):
+  ; NOAVX-NEXT:   [[V_SET0_:%[0-9]+]]:vr128 = V_SET0
+  ; NOAVX-NEXT:   [[MOV64rm:%[0-9]+]]:gr64 = MOV64rm $rip, 1, $noreg, target-flags(x86-gotpcrel) @z, $noreg :: (load (s64) from got, align 16)
+  ; NOAVX-NEXT:   MOVAPSmr killed [[MOV64rm]], 1, $noreg, 0, $noreg, killed [[V_SET0_]] :: (store (s128) into @z)
+  ; NOAVX-NEXT:   RET 0
+  ; AVX-LABEL: name: zero128
+  ; AVX: bb.0 (%ir-block.0):
+  ; AVX-NEXT:   [[V_SET0_:%[0-9]+]]:vr128 = V_SET0
+  ; AVX-NEXT:   [[MOV64rm:%[0-9]+]]:gr64 = MOV64rm $rip, 1, $noreg, target-flags(x86-gotpcrel) @z, $noreg :: (load (s64) from got, align 16)
+  ; AVX-NEXT:   VMOVAPSmr killed [[MOV64rm]], 1, $noreg, 0, $noreg, killed [[V_SET0_]] :: (store (s128) into @z)
+  ; AVX-NEXT:   RET 0
+  store <4 x float> zeroinitializer, ptr @z, align 16
+  ret void
+}
+

llvm/test/tools/UpdateTestChecks/update_mir_test_checks/x86-multiple-prefixes.test

+# REQUIRES: x86-registered-target
+## Check that update_mir_test_checks handles multiple check prefixes
+
+# RUN: cp -f %S/Inputs/x86-multiple-prefixes.ll %t.ll && %update_mir_test_checks %t.ll
+# RUN: diff -u %S/Inputs/x86-multiple-prefixes.ll.expected %t.ll

llvm/utils/update_mir_test_checks.py

for l in run_lines:
             for m in common.CHECK_PREFIX_RE.finditer(filecheck_cmd)
             for item in m.group(1).split(',')]
         if not check_prefixes:
             check_prefixes = ['CHECK']
         all_prefixes += check_prefixes
 
         run_list.append(Run(check_prefixes, cmd_args, triple))
 
-    # Remove any common prefixes. We'll just leave those entirely alone.
-    common_prefixes = set([prefix for prefix in all_prefixes
-                           if all_prefixes.count(prefix) > 1])
+    # Sort prefixes that are shared between run lines before unshared prefixes.
+    # This causes us to prefer printing shared prefixes.
     for run in run_list:
-        run.prefixes = [p for p in run.prefixes if p not in common_prefixes]
+        run.prefixes.sort(key=lambda prefix: -all_prefixes.count(prefix))
 
-    return run_list, common_prefixes
+    return run_list
 
 
 def find_functions_with_one_bb(lines, verbose=False):
     result = []
     cur_func = None
     bbs = 0
     for line in lines:
         m = MIR_FUNC_NAME_RE.match(line)
def build_function_body_dictionary(test, raw_tool_output, triple, prefixes,
                                    func_dict, verbose):
     for m in MIR_FUNC_RE.finditer(raw_tool_output):
         func = m.group('func')
         body = m.group('body')
         if verbose:
             log('Processing function: {}'.format(func))
             for l in body.splitlines():
                 log('  {}'.format(l))
+
+        # Vreg mangling
+        mangled = []
+        vreg_map = {}
+        for func_line in body.splitlines(keepends=True):
+            m = VREG_DEF_RE.match(func_line)
+            if m:
+                for vreg in VREG_RE.finditer(m.group('vregs')):
+                    name = mangle_vreg(m.group('opcode'), vreg_map.values())
+                    vreg_map[vreg.group(1)] = name
+                    func_line = func_line.replace(
+                        vreg.group(1), '[[{}:%[0-9]+]]'.format(name), 1)
+            for number, name in vreg_map.items():
+                func_line = re.sub(r'{}\b'.format(number), '[[{}]]'.format(name),
+                                func_line)
+            mangled.append(func_line)
+        body = ''.join(mangled)
+
         for prefix in prefixes:
-            if func in func_dict[prefix] and func_dict[prefix][func] != body:
-                common.warn('Found conflicting asm for prefix: {}'.format(prefix),
-                     test_file=test)
+            if func in func_dict[prefix]:
+                if func_dict[prefix][func] != body:
+                    func_dict[prefix][func] = None
+            else:
                 func_dict[prefix][func] = body
 
 
 def add_checks_for_function(test, output_lines, run_list, func_dict, func_name,
                             single_bb, verbose=False):
     printed_prefixes = set()
     for run in run_list:
         for prefix in run.prefixes:
             if prefix in printed_prefixes:
-                continue
+                break
             if not func_dict[prefix][func_name]:
                 continue
             # if printed_prefixes:
             #     # Add some space between different check prefixes.
             #     output_lines.append('')
             printed_prefixes.add(prefix)
             log('Adding {} lines for {}'.format(prefix, func_name), verbose)
             add_check_lines(test, output_lines, prefix, func_name, single_bb,
                             func_dict[prefix][func_name].splitlines())
             break
+        else:
+            common.warn(
+                'Found conflicting asm for function: {}'.format(func_name),
+                test_file=test)
     return output_lines
 
 
 def add_check_lines(test, output_lines, prefix, func_name, single_bb,
                     func_body):
     if single_bb:
         # Don't bother checking the basic block label for a single BB
         func_body.pop(0)
 
     if not func_body:
         common.warn('Function has no instructions to check: {}'.format(func_name),
              test_file=test)
         return
 
     first_line = func_body[0]
     indent = len(first_line) - len(first_line.lstrip(' '))
     # A check comment, indented the appropriate amount
     check = '{:>{}}; {}'.format('', indent, prefix)
 
     output_lines.append('{}-LABEL: name: {}'.format(check, func_name))
     first_check = True
 
-    vreg_map = {}
     for func_line in func_body:
         if not func_line.strip():
             # The mir printer prints leading whitespace so we can't use CHECK-EMPTY:
             output_lines.append(check + '-NEXT: {{' + func_line + '$}}')
             continue
-        m = VREG_DEF_RE.match(func_line)
-        if m:
-            for vreg in VREG_RE.finditer(m.group('vregs')):
-                name = mangle_vreg(m.group('opcode'), vreg_map.values())
-                vreg_map[vreg.group(1)] = name
-                func_line = func_line.replace(
-                    vreg.group(1), '[[{}:%[0-9]+]]'.format(name), 1)
-        for number, name in vreg_map.items():
-            func_line = re.sub(r'{}\b'.format(number), '[[{}]]'.format(name),
-                               func_line)
         filecheck_directive = check if first_check else check + '-NEXT'
         first_check = False
         check_line = '{}: {}'.format(filecheck_directive, func_line[indent:]).rstrip()
         output_lines.append(check_line)
 
 
 def mangle_vreg(opcode, current_names):
     base = opcode
def update_test_file(args, test):
 
     if args.update_only:
       if not first_line or 'autogenerated' not in first_line:
         common.warn("Skipping test which isn't autogenerated: " + test)
         return
 
     triple_in_ir = find_triple_in_ir(input_lines, args.verbose)
     run_lines = common.find_run_lines(test, input_lines)
-    run_list, common_prefixes = build_run_list(test, run_lines, args.verbose)
+    run_list = build_run_list(test, run_lines, args.verbose)
 
     simple_functions = find_functions_with_one_bb(input_lines, args.verbose)
 
     func_dict = {}
     for run in run_list:
         for prefix in run.prefixes:
             func_dict.update({prefix: dict()})
     for prefixes, llc_args, triple_in_cmd in run_list:
for prefixes, llc_args, triple_in_cmd in run_list:
                                        triple_in_cmd or triple_in_ir,
                                        prefixes, func_dict, args.verbose)
 
     state = 'toplevel'
     func_name = None
     prefix_set = set([prefix for run in run_list for prefix in run.prefixes])
     log('Rewriting FileCheck prefixes: {}'.format(prefix_set), args.verbose)
 
-    if args.remove_common_prefixes:
-        prefix_set.update(common_prefixes)
-    elif common_prefixes:
-        common.warn('Ignoring common prefixes: {}'.format(common_prefixes),
-             test_file=test)
-
     comment_char = '#' if test.endswith('.mir') else ';'
     autogenerated_note = ('{} NOTE: Assertions have been autogenerated by '
                           'utils/{}'.format(comment_char, script_name))
     output_lines = []
     output_lines.append(autogenerated_note)
 
     for input_line in input_lines:
         if input_line == autogenerated_note:
with open(test, 'wb') as fd:
         fd.writelines(['{}\n'.format(l).encode('utf-8') for l in output_lines])
 
 
 def main():
     parser = argparse.ArgumentParser(
         description=__doc__, formatter_class=argparse.RawTextHelpFormatter)
     parser.add_argument('--llc-binary', dest='llc', default='llc', type=LLC,
                         help='The "llc" binary to generate the test case with')
-    parser.add_argument('--remove-common-prefixes', action='store_true',
-                        help='Remove existing check lines whose prefixes are '
-                             'shared between multiple commands')
     parser.add_argument('tests', nargs='+')
     args = common.parse_commandline_args(parser)
 
     test_paths = [test for pattern in args.tests for test in glob.glob(pattern)]
     for test in test_paths:
         try:
             update_test_file(args, test)
         except Exception:
             common.warn('Error processing file', test_file=test)
             raise
 
 
 if __name__ == '__main__':
   main()