[update_llc_test_checks] Use FileCheck captures for MCInst/MCReg output

To avoid test churn when backends add/rename new instructions/registers,
it makes sense to use FileCheck captures for the exact MCInst/Reg number.
This is motivated by D125307, where I use --asm-show-inst to differentiate
the output for multiple instructions with the same mnemonic.

This does not quite fix the churn issue yet: While files with the generated
checks will be immune to the numbers changing, the update script test
still suffers from this problem since the number is encoded in the
FileCheck variable name. I plan to address this in a follow-up patch.

Reviewed By: MaskRay

Differential Revision: https://reviews.llvm.org/D125307

llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/asm-show-inst.ll.expected

@@ -6,17 +6,17 @@
 define i8 @add_i8(i8 %a) nounwind {
 ; VERBOSE-LABEL: add_i8:
 ; VERBOSE:       # %bb.0:
-; VERBOSE-NEXT:    movb {{[0-9]+}}(%esp), %al # <MCInst #1804 MOV8rm
-; VERBOSE-NEXT:    # <MCOperand Reg:2>
-; VERBOSE-NEXT:    # <MCOperand Reg:33>
+; VERBOSE-NEXT:    movb {{[0-9]+}}(%esp), %al # <MCInst #[[#MCINST1804:]] MOV8rm
+; VERBOSE-NEXT:    # <MCOperand Reg:[[#MCREG2:]]>
+; VERBOSE-NEXT:    # <MCOperand Reg:[[#MCREG33:]]>
 ; VERBOSE-NEXT:    # <MCOperand Imm:1>
-; VERBOSE-NEXT:    # <MCOperand Reg:0>
+; VERBOSE-NEXT:    # <MCOperand Reg:[[#MCREG0:]]>
 ; VERBOSE-NEXT:    # <MCOperand Imm:4>
-; VERBOSE-NEXT:    # <MCOperand Reg:0>>
-; VERBOSE-NEXT:    addb $2, %al # <MCInst #400 ADD8i8
+; VERBOSE-NEXT:    # <MCOperand Reg:[[#MCREG0]]>>
+; VERBOSE-NEXT:    addb $2, %al # <MCInst #[[#MCINST400:]] ADD8i8
 ; VERBOSE-NEXT:    # <MCOperand Imm:2>>
-; VERBOSE-NEXT:    retl # <MCInst #2560 RET32
-; VERBOSE-NEXT:    # <MCOperand Reg:2>>
+; VERBOSE-NEXT:    retl # <MCInst #[[#MCINST2560:]] RET32
+; VERBOSE-NEXT:    # <MCOperand Reg:[[#MCREG2]]>>
 ;
 ; CHECK-LABEL: add_i8:
 ; CHECK:       # %bb.0:
@@ -30,19 +30,19 @@ define i8 @add_i8(i8 %a) nounwind {
 define i32 @add_i32(i32 %a) nounwind {
 ; VERBOSE-LABEL: add_i32:
 ; VERBOSE:       # %bb.0:
-; VERBOSE-NEXT:    movl {{[0-9]+}}(%esp), %eax # <MCInst #1768 MOV32rm
-; VERBOSE-NEXT:    # <MCOperand Reg:22>
-; VERBOSE-NEXT:    # <MCOperand Reg:33>
+; VERBOSE-NEXT:    movl {{[0-9]+}}(%esp), %eax # <MCInst #[[#MCINST1768:]] MOV32rm
+; VERBOSE-NEXT:    # <MCOperand Reg:[[#MCREG22:]]>
+; VERBOSE-NEXT:    # <MCOperand Reg:[[#MCREG33]]>
 ; VERBOSE-NEXT:    # <MCOperand Imm:1>
-; VERBOSE-NEXT:    # <MCOperand Reg:0>
+; VERBOSE-NEXT:    # <MCOperand Reg:[[#MCREG0]]>
 ; VERBOSE-NEXT:    # <MCOperand Imm:4>
-; VERBOSE-NEXT:    # <MCOperand Reg:0>>
-; VERBOSE-NEXT:    addl $2, %eax # <MCInst #387 ADD32ri8
-; VERBOSE-NEXT:    # <MCOperand Reg:22>
-; VERBOSE-NEXT:    # <MCOperand Reg:22>
+; VERBOSE-NEXT:    # <MCOperand Reg:[[#MCREG0]]>>
+; VERBOSE-NEXT:    addl $2, %eax # <MCInst #[[#MCINST387:]] ADD32ri8
+; VERBOSE-NEXT:    # <MCOperand Reg:[[#MCREG22]]>
+; VERBOSE-NEXT:    # <MCOperand Reg:[[#MCREG22]]>
 ; VERBOSE-NEXT:    # <MCOperand Imm:2>>
-; VERBOSE-NEXT:    retl # <MCInst #2560 RET32
-; VERBOSE-NEXT:    # <MCOperand Reg:22>>
+; VERBOSE-NEXT:    retl # <MCInst #[[#MCINST2560]] RET32
+; VERBOSE-NEXT:    # <MCOperand Reg:[[#MCREG22]]>>
 ;
 ; CHECK-LABEL: add_i32:
 ; CHECK:       # %bb.0:

llvm/utils/UpdateTestChecks/asm.py

@@ -423,9 +423,8 @@ def scrub_asm_nvptx(asm, args):
   asm = common.SCRUB_TRAILING_WHITESPACE_RE.sub(r'', asm)
   return asm
 
-
 # Returns a tuple of a scrub function and a function regex. Scrub function is
-# used to alter function body in some way, for example, remove traling spaces.
+# used to alter function body in some way, for example, remove trailing spaces.
 # Function regex is used to match function name, body, etc. in raw llc output.
 def get_run_handler(triple):
   target_handlers = {
@@ -482,10 +481,9 @@ def get_run_handler(triple):
 ##### Generator of assembly CHECK lines
 
 def add_checks(output_lines, comment_marker, prefix_list, func_dict,
-                   func_name, is_filtered):
+               func_name, global_vars_seen_dict, is_filtered):
   # Label format is based on ASM string.
   check_label_format = '{} %s-LABEL: %s%s%s'.format(comment_marker)
-  global_vars_seen_dict = {}
   common.add_checks(output_lines, comment_marker, prefix_list, func_dict,
                     func_name, check_label_format, True, False,
-                    global_vars_seen_dict, is_filtered = is_filtered)
+                    global_vars_seen_dict, is_filtered=is_filtered)

llvm/utils/UpdateTestChecks/common.py

@@ -3,6 +3,7 @@
 import argparse
 import copy
 import glob
+import itertools
 import os
 import re
 import subprocess
@@ -596,7 +597,7 @@ def _get_failed_prefixes(self):
 
 class NamelessValue:
   def __init__(self, check_prefix, check_key, ir_prefix, global_ir_prefix, global_ir_prefix_regexp,
-               ir_regexp, global_ir_rhs_regexp, is_before_functions):
+               ir_regexp, global_ir_rhs_regexp, is_before_functions, is_number=False):
     self.check_prefix = check_prefix
     self.check_key = check_key
     self.ir_prefix = ir_prefix
@@ -605,6 +606,7 @@ def __init__(self, check_prefix, check_key, ir_prefix, global_ir_prefix, global_
     self.ir_regexp = ir_regexp
     self.global_ir_rhs_regexp = global_ir_rhs_regexp
     self.is_before_functions = is_before_functions
+    self.is_number = is_number
 
   # Return true if this kind of IR value is "local", basically if it matches '%{{.*}}'.
   def is_local_def_ir_value_match(self, match):
@@ -635,23 +637,29 @@ def get_value_definition(self, var, match):
     # for backwards compatibility we check locals with '.*'
     varname = get_value_name(var, self.check_prefix)
     prefix = self.get_ir_prefix_from_ir_value_match(match)[0]
-    regex = self.get_ir_regex_from_ir_value_re_match(match)
+    if self.is_number:
+      regex = ''  # always capture a number in the default format
+      capture_start = '[[#'
+    else:
+      regex = self.get_ir_regex_from_ir_value_re_match(match)
+      capture_start = '[['
     if self.is_local_def_ir_value_match(match):
-      return '[[' + varname + ':' + prefix + regex + ']]'
-    return prefix + '[[' + varname + ':' + regex + ']]'
+      return capture_start + varname + ':' + prefix + regex + ']]'
+    return prefix + capture_start + varname + ':' + regex + ']]'
 
   # Use a FileCheck variable.
   def get_value_use(self, var, match, var_prefix=None):
     if var_prefix is None:
       var_prefix = self.check_prefix
+    capture_start = '[[#' if self.is_number else '[['
     if self.is_local_def_ir_value_match(match):
-      return '[[' + get_value_name(var, var_prefix) + ']]'
+      return capture_start + get_value_name(var, var_prefix) + ']]'
     prefix = self.get_ir_prefix_from_ir_value_match(match)[0]
-    return prefix + '[[' + get_value_name(var, var_prefix) + ']]'
+    return prefix + capture_start + get_value_name(var, var_prefix) + ']]'
 
 # Description of the different "unnamed" values we match in the IR, e.g.,
 # (local) ssa values, (debug) metadata, etc.
-nameless_values = [
+ir_nameless_values = [
     NamelessValue(r'TMP'  , '%' , r'%'           , None            , None                   , r'[\w$.-]+?' , None                 , False) ,
     NamelessValue(r'ATTR' , '#' , r'#'           , None            , None                   , r'[0-9]+'    , None                 , False) ,
     NamelessValue(r'ATTR' , '#' , None           , r'attributes #' , r'[0-9]+'              , None         , r'{[^}]*}'           , False) ,
@@ -666,6 +674,11 @@ def get_value_use(self, var, match, var_prefix=None):
     NamelessValue(r'META' , '!' , None           , r''             , r'![0-9]+'             , None         , r'(?:distinct |)!.*' , False) ,
 ]
 
+asm_nameless_values = [
+ NamelessValue(r'MCINST', 'Inst#', None, '<MCInst #', r'\d+', None, r'.+', False, True),
+ NamelessValue(r'MCREG',  'Reg:', None, '<MCOperand Reg:', r'\d+', None, r'.+', False, True),
+]
+
 def createOrRegexp(old, new):
   if not old:
     return new
@@ -684,7 +697,7 @@ def createPrefixMatch(prefix_str, prefix_re):
 # other locations will need adjustment as well.
 IR_VALUE_REGEXP_PREFIX = r'(\s*)'
 IR_VALUE_REGEXP_STRING = r''
-for nameless_value in nameless_values:
+for nameless_value in ir_nameless_values:
   lcl_match = createPrefixMatch(nameless_value.ir_prefix, nameless_value.ir_regexp)
   glb_match = createPrefixMatch(nameless_value.global_ir_prefix, nameless_value.global_ir_prefix_regexp)
   assert((lcl_match or glb_match) and not (lcl_match and glb_match))
@@ -695,6 +708,15 @@ def createPrefixMatch(prefix_str, prefix_re):
 IR_VALUE_REGEXP_SUFFIX = r'([,\s\(\)]|\Z)'
 IR_VALUE_RE = re.compile(IR_VALUE_REGEXP_PREFIX + r'(' + IR_VALUE_REGEXP_STRING + r')' + IR_VALUE_REGEXP_SUFFIX)
 
+# Build the regexp that matches an "ASM value" (currently only for --asm-show-inst comments).
+ASM_VALUE_REGEXP_STRING = ''
+for nameless_value in asm_nameless_values:
+  glb_match = createPrefixMatch(nameless_value.global_ir_prefix, nameless_value.global_ir_prefix_regexp)
+  assert not nameless_value.ir_prefix and not nameless_value.ir_regexp
+  ASM_VALUE_REGEXP_STRING = createOrRegexp(ASM_VALUE_REGEXP_STRING, glb_match)
+ASM_VALUE_REGEXP_SUFFIX = r'([>\s]|\Z)'
+ASM_VALUE_RE = re.compile(r'((?:#|//)\s*)' + '(' + ASM_VALUE_REGEXP_STRING + ')' + ASM_VALUE_REGEXP_SUFFIX)
+
 # The entire match is group 0, the prefix has one group (=1), the entire
 # IR_VALUE_REGEXP_STRING is one group (=2), and then the nameless values start.
 first_nameless_group_in_ir_value_match = 3
@@ -738,7 +760,9 @@ def get_value_name(var, check_prefix):
   var = var.replace('-', '_')
   return var.upper()
 
-def generalize_check_lines(lines, is_analyze, vars_seen, global_vars_seen):
+def generalize_check_lines_common(lines, is_analyze, vars_seen,
+                                  global_vars_seen, nameless_values,
+                                  nameless_value_regex, is_asm):
   # This gets called for each match that occurs in
   # a line. We transform variables we haven't seen
   # into defs, and variables we have seen into uses.
@@ -771,26 +795,38 @@ def transform_line_vars(match):
   lines_with_def = []
 
   for i, line in enumerate(lines):
-    # An IR variable named '%.' matches the FileCheck regex string.
-    line = line.replace('%.', '%dot')
-    for regex in _global_hex_value_regex:
-      if re.match('^@' + regex + ' = ', line):
-        line = re.sub(r'\bi([0-9]+) ([0-9]+)',
-            lambda m : 'i' + m.group(1) + ' [[#' + hex(int(m.group(2))) + ']]',
-            line)
-        break
-    # Ignore any comments, since the check lines will too.
-    scrubbed_line = SCRUB_IR_COMMENT_RE.sub(r'', line)
-    lines[i] = scrubbed_line
-    if not is_analyze:
+    if not is_asm:
+      # An IR variable named '%.' matches the FileCheck regex string.
+      line = line.replace('%.', '%dot')
+      for regex in _global_hex_value_regex:
+        if re.match('^@' + regex + ' = ', line):
+          line = re.sub(r'\bi([0-9]+) ([0-9]+)',
+              lambda m : 'i' + m.group(1) + ' [[#' + hex(int(m.group(2))) + ']]',
+              line)
+          break
+      # Ignore any comments, since the check lines will too.
+      scrubbed_line = SCRUB_IR_COMMENT_RE.sub(r'', line)
+      lines[i] = scrubbed_line
+    if is_asm or not is_analyze:
       # It can happen that two matches are back-to-back and for some reason sub
       # will not replace both of them. For now we work around this by
       # substituting until there is no more match.
       changed = True
       while changed:
-        (lines[i], changed) = IR_VALUE_RE.subn(transform_line_vars, lines[i], count=1)
+        (lines[i], changed) = nameless_value_regex.subn(transform_line_vars,
+                                                        lines[i], count=1)
   return lines
 
+# Replace IR value defs and uses with FileCheck variables.
+def generalize_check_lines(lines, is_analyze, vars_seen, global_vars_seen):
+  return generalize_check_lines_common(lines, is_analyze, vars_seen,
+                                       global_vars_seen, ir_nameless_values,
+                                       IR_VALUE_RE, False)
+
+def generalize_asm_check_lines(lines, vars_seen, global_vars_seen):
+  return generalize_check_lines_common(lines, False, vars_seen,
+                                       global_vars_seen, asm_nameless_values,
+                                       ASM_VALUE_RE, True)
 
 def add_checks(output_lines, comment_marker, prefix_list, func_dict, func_name, check_label_format, is_backend, is_analyze, global_vars_seen_dict, is_filtered):
   # prefix_exclusions are prefixes we cannot use to print the function because it doesn't exist in run lines that use these prefixes as well.
@@ -843,7 +879,8 @@ def add_checks(output_lines, comment_marker, prefix_list, func_dict, func_name,
       if attrs:
         output_lines.append('%s %s: Function Attrs: %s' % (comment_marker, checkprefix, attrs))
       args_and_sig = str(func_dict[checkprefix][func_name].args_and_sig)
-      args_and_sig = generalize_check_lines([args_and_sig], is_analyze, vars_seen, global_vars_seen)[0]
+      if args_and_sig:
+        args_and_sig = generalize_check_lines([args_and_sig], is_analyze, vars_seen, global_vars_seen)[0]
       func_name_separator = func_dict[checkprefix][func_name].func_name_separator
       if '[[' in args_and_sig:
         output_lines.append(check_label_format % (checkprefix, func_name, '', func_name_separator))
@@ -864,13 +901,19 @@ def add_checks(output_lines, comment_marker, prefix_list, func_dict, func_name,
           body_start = 0
         else:
           output_lines.append('%s %s:       %s' % (comment_marker, checkprefix, func_body[0]))
-        for func_line in func_body[body_start:]:
+        func_lines = generalize_asm_check_lines(func_body[body_start:],
+                                                vars_seen, global_vars_seen)
+        for func_line in func_lines:
           if func_line.strip() == '':
             output_lines.append('%s %s-EMPTY:' % (comment_marker, checkprefix))
           else:
             check_suffix = '-NEXT' if not is_filtered else ''
             output_lines.append('%s %s%s:  %s' % (comment_marker, checkprefix,
                                                   check_suffix, func_line))
+        # Remember new global variables we have not seen before
+        for key in global_vars_seen:
+          if key not in global_vars_seen_before:
+            global_vars_seen_dict[checkprefix][key] = global_vars_seen[key]
         break
 
       # For IR output, change all defs to FileCheck variables, so we're immune
@@ -911,7 +954,7 @@ def add_checks(output_lines, comment_marker, prefix_list, func_dict, func_name,
       # line of code in the test function.
       output_lines.append(comment_marker)
 
-      # Remembe new global variables we have not seen before
+      # Remember new global variables we have not seen before
       for key in global_vars_seen:
         if key not in global_vars_seen_before:
           global_vars_seen_dict[checkprefix][key] = global_vars_seen[key]
@@ -935,7 +978,7 @@ def add_analyze_checks(output_lines, comment_marker, prefix_list, func_dict, fun
              is_filtered)
 
 def build_global_values_dictionary(glob_val_dict, raw_tool_output, prefixes):
-  for nameless_value in nameless_values:
+  for nameless_value in itertools.chain(ir_nameless_values, asm_nameless_values):
     if nameless_value.global_ir_prefix is None:
       continue
 
@@ -963,7 +1006,7 @@ def build_global_values_dictionary(glob_val_dict, raw_tool_output, prefixes):
 
 def add_global_checks(glob_val_dict, comment_marker, prefix_list, output_lines, global_vars_seen_dict, is_analyze, is_before_functions):
   printed_prefixes = set()
-  for nameless_value in nameless_values:
+  for nameless_value in ir_nameless_values:
     if nameless_value.global_ir_prefix is None:
       continue
     if nameless_value.is_before_functions != is_before_functions:

llvm/utils/UpdateTestChecks/isel.py

@@ -48,10 +48,10 @@ def get_run_handler(triple):
 
 ##### Generator of iSel CHECK lines
 
-def add_checks(output_lines, comment_marker, prefix_list, func_dict, func_name, is_filtered):
+def add_checks(output_lines, comment_marker, prefix_list, func_dict, func_name,
+               global_vars_seen_dict, is_filtered):
   # Label format is based on iSel string.
   check_label_format = '{} %s-LABEL: %s%s%s'.format(comment_marker)
-  global_vars_seen_dict = {}
   common.add_checks(output_lines, comment_marker, prefix_list, func_dict,
                     func_name, check_label_format, True, False,
                     global_vars_seen_dict, is_filtered = is_filtered)

llvm/utils/update_cc_test_checks.py

@@ -349,7 +349,7 @@ def check_generator(my_output_lines, prefixes, func):
         else:
           asm.add_checks(my_output_lines, '//',
                              prefixes,
-                             func_dict, func,
+                             func_dict, func, global_vars_seen_dict,
                              is_filtered=builder.is_filtered())
 
       if ti.args.check_globals:

llvm/utils/update_llc_test_checks.py

@@ -139,6 +139,7 @@ def main():
       builder.process_run_line(function_re, scrubber, raw_tool_output, prefixes, True)
 
     func_dict = builder.finish_and_get_func_dict()
+    global_vars_seen_dict = {}
 
     is_in_function = False
     is_in_function_start = False
@@ -169,6 +170,7 @@ def main():
                                output_type.add_checks(my_output_lines,
                                                   check_indent + ';',
                                                   prefixes, func_dict, func,
+                                                  global_vars_seen_dict,
                                                   is_filtered=builder.is_filtered()))
     else:
       for input_info in ti.iterlines(output_lines):
@@ -185,8 +187,8 @@ def main():
 
           # Print out the various check lines here.
           output_type.add_checks(output_lines, check_indent + ';', run_list,
-                             func_dict, func_name,
-                             is_filtered=builder.is_filtered())
+                                 func_dict, func_name, global_vars_seen_dict,
+                                 is_filtered=builder.is_filtered())
           is_in_function_start = False
 
         if is_in_function: