[UTC] Include return type/attributes under --version 2

[nikic]

Summary

If --function-signature is used with --version 2, then also include the return type/attributes in the check lines. This is the implementation of D133943 rebased on the --version mechanism from D142473.

This doesn't bump the default version yet, because I'd like to do that together with D140212 (which enables --function-signature by default), as these changes seem closely related. For now this functionality can be accessed by explicitly passing --version 2 to UTC.

Fixes https://github.com/llvm/llvm-project/issues/61058.

[arichardson]

LGTM

[asb]

Firstly - thanks for picking this up again - I just came back to some work that would benefit, so was about to look myself. I'm grateful you've already done that rebasing!

This LGTM in general. I have just one question about how you imagine changing the incorporation of D140212 - would you add in the --function-signature by default change to --version 2, or introduce a --version 3? I think I'd be happy enough with either, but wanted to check your plan.

[nikic]

In D144963#4158739, @asb wrote:

This LGTM in general. I have just one question about how you imagine changing the incorporation of D140212 - would you add in the --function-signature by default change to --version 2, or introduce a --version 3? I think I'd be happy enough with either, but wanted to check your plan.

My plan was to both enable --function-signature by default and check return types as part of --version 2. But I would be fine keeping them separate as well (in which case I'd bump the default version in this patch).

[asb]

In D144963#4158771, @nikic wrote:

In D144963#4158739, @asb wrote:

This LGTM in general. I have just one question about how you imagine changing the incorporation of D140212 - would you add in the --function-signature by default change to --version 2, or introduce a --version 3? I think I'd be happy enough with either, but wanted to check your plan.

My plan was to both enable --function-signature by default and check return types as part of --version 2. But I would be fine keeping them separate as well (in which case I'd bump the default version in this patch).

I thought I'd flag it in case others felt strongly otherwise, but doing it as part of --version 2 seems fine to me. It might seem a bit weird if you see --version as an attempt to give an immutable output format for UTC at a point in time. But if you see it as just a tool to help reduce in-tree test churn when adding new features, then logically we have the option of extending the current "version" as an alternative to defining a new one, if it's not too disruptive to do so.

[asb]

I've rebased D134050 and D140400 on top of this.

clang/test/utils/update_cc_test_checks/Inputs/mangled_names.c.funcsig.v2.expected

+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --function-signature --version 2
+// Example input for update_cc_test_checks
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: define dso_local i64 @test
+// CHECK-SAME: (i64 noundef [[A:%.*]], i32 noundef [[B:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[A_ADDR:%.*]] = alloca i64, align 8
+// CHECK-NEXT:    [[B_ADDR:%.*]] = alloca i32, align 4
+// CHECK-NEXT:    store i64 [[A]], ptr [[A_ADDR]], align 8
+// CHECK-NEXT:    store i32 [[B]], ptr [[B_ADDR]], align 4
+// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr [[A_ADDR]], align 8
+// CHECK-NEXT:    [[TMP1:%.*]] = load i32, ptr [[B_ADDR]], align 4
+// CHECK-NEXT:    [[CONV:%.*]] = sext i32 [[TMP1]] to i64
+// CHECK-NEXT:    [[ADD:%.*]] = add nsw i64 [[TMP0]], [[CONV]]
+// CHECK-NEXT:    ret i64 [[ADD]]
+//
+long test(long a, int b) {
+  return a + b;
+}
+
+// A function with a mangled name
+// CHECK-LABEL: define dso_local i64 @_Z4testlii
+// CHECK-SAME: (i64 noundef [[A:%.*]], i32 noundef [[B:%.*]], i32 noundef [[C:%.*]]) #[[ATTR0]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[A_ADDR:%.*]] = alloca i64, align 8
+// CHECK-NEXT:    [[B_ADDR:%.*]] = alloca i32, align 4
+// CHECK-NEXT:    [[C_ADDR:%.*]] = alloca i32, align 4
+// CHECK-NEXT:    store i64 [[A]], ptr [[A_ADDR]], align 8
+// CHECK-NEXT:    store i32 [[B]], ptr [[B_ADDR]], align 4
+// CHECK-NEXT:    store i32 [[C]], ptr [[C_ADDR]], align 4
+// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr [[A_ADDR]], align 8
+// CHECK-NEXT:    [[TMP1:%.*]] = load i32, ptr [[B_ADDR]], align 4
+// CHECK-NEXT:    [[CONV:%.*]] = sext i32 [[TMP1]] to i64
+// CHECK-NEXT:    [[ADD:%.*]] = add nsw i64 [[TMP0]], [[CONV]]
+// CHECK-NEXT:    [[TMP2:%.*]] = load i32, ptr [[C_ADDR]], align 4
+// CHECK-NEXT:    [[CONV1:%.*]] = sext i32 [[TMP2]] to i64
+// CHECK-NEXT:    [[ADD2:%.*]] = add nsw i64 [[ADD]], [[CONV1]]
+// CHECK-NEXT:    ret i64 [[ADD2]]
+//
+__attribute__((overloadable)) long test(long a, int b, int c) {
+  return a + b + c;
+}

clang/test/utils/update_cc_test_checks/mangled_names.test

 ## Running it again should implicitly add the function-signature flag due to UTC_ARGS:
 # RUN: %update_cc_test_checks %t.c
 # RUN: diff -u %t.c %S/Inputs/mangled_names.c.funcsig.expected
 ## Verify that running without the --function-signature flag removes the -SAME: lines:
 ## We have to remove the UTC_ARGS comment first:
 # RUN: grep -v UTC_ARGS %t.c > %t-no-args.c
 # RUN: %update_cc_test_checks %t-no-args.c
 # RUN: diff -u %t-no-args.c %S/Inputs/mangled_names.c.expected
+## Also try --version 2
+# RUN: %update_cc_test_checks %t.c --function-signature --version 2
+# RUN: diff -u %t.c %S/Inputs/mangled_names.c.funcsig.v2.expected

llvm/utils/UpdateTestChecks/asm.py

def get_run_handler(triple):
 
   return handler
 
 ##### Generator of assembly CHECK lines
 
 def add_checks(output_lines, comment_marker, prefix_list, func_dict,
                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)
+  check_label_format = '{} %s-LABEL: %s%s%s%s'.format(comment_marker)
   return common.add_checks(output_lines, comment_marker, prefix_list, func_dict,
-                           func_name, check_label_format, True, False,
+                           func_name, check_label_format, True, False, 1,
                            global_vars_seen_dict, is_filtered=is_filtered)

llvm/utils/UpdateTestChecks/common.py

 
 _verbose = False
 _prefix_filecheck_ir_name = ''
 
 """
 Version changelog:
 
 1: Initial version, used by tests that don't specify --version explicitly.
+2: --function-signature now also checks return type/attributes.
 """
 DEFAULT_VERSION = 1
 
 class Regex(object):
   """Wrap a compiled regular expression object to allow deep copy of a regexp.
   This is required for the deep copy done in do_scrub.
 
   """

 CHECK_RE = re.compile(r'^\s*(?://|[;#])\s*([^:]+?)(?:-NEXT|-NOT|-DAG|-LABEL|-SAME|-EMPTY)?:')
 
 UTC_ARGS_KEY = 'UTC_ARGS:'
 UTC_ARGS_CMD = re.compile(r'.*' + UTC_ARGS_KEY + '\s*(?P<cmd>.*)\s*$')
 UTC_ADVERT = 'NOTE: Assertions have been autogenerated by '
 UNUSED_NOTE = 'NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:'
 
 OPT_FUNCTION_RE = re.compile(
-    r'^(\s*;\s*Function\sAttrs:\s(?P<attrs>[\w\s():,]+?))?\s*define\s+(?:internal\s+)?[^@]*@(?P<func>[\w.$-]+?)\s*'
+    r'^(\s*;\s*Function\sAttrs:\s(?P<attrs>[\w\s():,]+?))?\s*define\s+(?P<funcdef_attrs_and_ret>[^@]*)@(?P<func>[\w.$-]+?)\s*'
     r'(?P<args_and_sig>\((\)|(.*?[\w.-]+?)\))[^{]*\{)\n(?P<body>.*?)^\}$',
     flags=(re.M | re.S))
 
 ANALYZE_FUNCTION_RE = re.compile(
     r'^\s*\'(?P<analysis>[\w\s-]+?)\'\s+for\s+function\s+\'(?P<func>[\w.$-]+?)\':'
     r'\s*\n(?P<body>.*)$',
     flags=(re.X | re.S))
 
def do_scrub(body, scrubber, scrubber_args, extra):
   if scrubber_args:
     local_args = copy.deepcopy(scrubber_args)
     local_args[0].extra_scrub = extra
     return scrubber(body, *local_args)
   return scrubber(body, *scrubber_args)
 
 # Build up a dictionary of all the function bodies.
 class function_body(object):
-  def __init__(self, string, extra, args_and_sig, attrs, func_name_separator):
+  def __init__(self, string, extra, funcdef_attrs_and_ret, args_and_sig, attrs, func_name_separator):
     self.scrub = string
     self.extrascrub = extra
+    self.funcdef_attrs_and_ret = funcdef_attrs_and_ret
     self.args_and_sig = args_and_sig
     self.attrs = attrs
     self.func_name_separator = func_name_separator
-  def is_same_except_arg_names(self, extrascrub, args_and_sig, attrs, is_backend):
+  def is_same_except_arg_names(self, extrascrub, funcdef_attrs_and_ret, args_and_sig, attrs, is_backend):
     arg_names = set()
     def drop_arg_names(match):
       arg_names.add(match.group(variable_group_in_ir_value_match))
       if match.group(attribute_group_in_ir_value_match):
         attr = match.group(attribute_group_in_ir_value_match)
       else:
         attr = ''
       return match.group(1) + attr + match.group(match.lastindex)
     def repl_arg_names(match):
       if match.group(variable_group_in_ir_value_match) is not None and match.group(variable_group_in_ir_value_match) in arg_names:
         return match.group(1) + match.group(match.lastindex)
       return match.group(1) + match.group(2) + match.group(match.lastindex)
+    if self.funcdef_attrs_and_ret != funcdef_attrs_and_ret:
+      return False
     if self.attrs != attrs:
       return False
     ans0 = IR_VALUE_RE.sub(drop_arg_names, self.args_and_sig)
     ans1 = IR_VALUE_RE.sub(drop_arg_names, args_and_sig)
     if ans0 != ans1:
       return False
     if is_backend:
       # Check without replacements, the replacements are not applied to the
for m in function_re.finditer(raw_tool_output):
       # colon: `foo:`. But, for example, in nvptx it is a brace: `foo(`. If is_backend is
       # False, just assume that separator is an empty string.
       if is_backend:
         # Use ':' as default separator.
         func_name_separator = m.group('func_name_separator') if 'func_name_separator' in m.groupdict() else ':'
       else:
         func_name_separator = ''
       attrs = m.group('attrs') if self._check_attributes else ''
+      funcdef_attrs_and_ret = m.group('funcdef_attrs_and_ret') if self._record_args else ''
       # Determine if we print arguments, the opening brace, or nothing after the
       # function name
       if self._record_args and 'args_and_sig' in m.groupdict():
         args_and_sig = scrub_body(m.group('args_and_sig').strip())
       elif 'args_and_sig' in m.groupdict():
         args_and_sig = '('
       else:
         args_and_sig = ''
for m in function_re.finditer(raw_tool_output):
             # capture groups set.
             scrubbed_body = re.sub(func_repl, '{{' + func_repl + '}}',
                                    scrubbed_body)
 
         if func in self._func_dict[prefix]:
           if (self._func_dict[prefix][func] is not None and
               (str(self._func_dict[prefix][func]) != scrubbed_body or
                self._func_dict[prefix][func].args_and_sig != args_and_sig or
-               self._func_dict[prefix][func].attrs != attrs)):
+               self._func_dict[prefix][func].attrs != attrs or
+               self._func_dict[prefix][func].funcdef_attrs_and_ret != funcdef_attrs_and_ret)):
             if self._func_dict[prefix][func].is_same_except_arg_names(
                 scrubbed_extra,
+                funcdef_attrs_and_ret,
                 args_and_sig,
                 attrs,
                 is_backend):
               self._func_dict[prefix][func].scrub = scrubbed_extra
               self._func_dict[prefix][func].args_and_sig = args_and_sig
             else:
               # This means a previous RUN line produced a body for this function
               # that is different from the one produced by this current RUN line,
               # so the body can't be common across RUN lines. We use None to
               # indicate that.
               self._func_dict[prefix][func] = None
         else:
           if prefix not in self._processed_prefixes:
             self._func_dict[prefix][func] = function_body(
-                scrubbed_body, scrubbed_extra, args_and_sig, attrs,
-                func_name_separator)
+                scrubbed_body, scrubbed_extra, funcdef_attrs_and_ret,
+                args_and_sig, attrs, func_name_separator)
             self._func_order[prefix].append(func)
           else:
             # An earlier RUN line used this check prefixes but didn't produce
             # a body for this function. This happens in Clang tests that use
             # preprocesser directives to exclude individual functions from some
             # RUN lines.
             self._func_dict[prefix][func] = None
 
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):
+def add_checks(output_lines, comment_marker, prefix_list, func_dict, func_name, check_label_format, is_backend, is_analyze, version, 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.
   prefix_exclusions = set()
   printed_prefixes = []
   for p in prefix_list:
     checkprefixes = p[0]
     # If not all checkprefixes of this run line produced the function we cannot check for it as it does not
     # exist for this run line. A subset of the check prefixes might know about the function but only because
     # other run lines created it.
for checkprefix in checkprefixes:
         global_vars_seen_dict[checkprefix] = {}
 
       global_vars_seen_before = [key for key in global_vars_seen.keys()]
 
       vars_seen = set()
       printed_prefixes.append(checkprefix)
       attrs = str(func_dict[checkprefix][func_name].attrs)
       attrs = '' if attrs == 'None' else attrs
+      if version > 1:
+        funcdef_attrs_and_ret = func_dict[checkprefix][func_name].funcdef_attrs_and_ret
+      else:
+        funcdef_attrs_and_ret = ''
+
       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)
       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))
+        output_lines.append(check_label_format % (checkprefix, funcdef_attrs_and_ret, func_name, '', func_name_separator))
         output_lines.append('%s %s-SAME: %s' % (comment_marker, checkprefix, args_and_sig))
       else:
-        output_lines.append(check_label_format % (checkprefix, func_name, args_and_sig, func_name_separator))
+        output_lines.append(check_label_format % (checkprefix, funcdef_attrs_and_ret, func_name, args_and_sig, func_name_separator))
       func_body = str(func_dict[checkprefix][func_name]).splitlines()
       if not func_body:
         # We have filtered everything.
         continue
 
       # For ASM output, just emit the check lines.
       if is_backend:
         body_start = 1
for checkprefix in checkprefixes:
       # 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
   return printed_prefixes
 
 def add_ir_checks(output_lines, comment_marker, prefix_list, func_dict,
-                  func_name, preserve_names, function_sig,
+                  func_name, preserve_names, function_sig, version,
                   global_vars_seen_dict, is_filtered):
   # Label format is based on IR string.
-  function_def_regex = 'define {{[^@]+}}' if function_sig else ''
+  if function_sig and version > 1:
+    function_def_regex = 'define %s'
+  elif function_sig:
+    function_def_regex = 'define {{[^@]+}}%s'
+  else:
+    function_def_regex = '%s'
   check_label_format = '{} %s-LABEL: {}@%s%s%s'.format(comment_marker, function_def_regex)
   return add_checks(output_lines, comment_marker, prefix_list, func_dict, func_name,
-                    check_label_format, False, preserve_names, global_vars_seen_dict,
+                    check_label_format, False, preserve_names, version,
+                    global_vars_seen_dict,
                     is_filtered)
 
 def add_analyze_checks(output_lines, comment_marker, prefix_list, func_dict, func_name, is_filtered):
-  check_label_format = '{} %s-LABEL: \'%s%s%s\''.format(comment_marker)
+  check_label_format = '{} %s-LABEL: \'%s%s%s%s\''.format(comment_marker)
   global_vars_seen_dict = {}
   return add_checks(output_lines, comment_marker, prefix_list, func_dict, func_name,
-                    check_label_format, False, True, global_vars_seen_dict,
+                    check_label_format, False, True, 1, global_vars_seen_dict,
                     is_filtered)
 
 def build_global_values_dictionary(glob_val_dict, raw_tool_output, prefixes):
   for nameless_value in itertools.chain(ir_nameless_values, asm_nameless_values):
     if nameless_value.global_ir_rhs_regexp is None:
       continue
 
     lhs_re_str = nameless_value.ir_prefix + nameless_value.ir_regexp

llvm/utils/UpdateTestChecks/isel.py

def get_run_handler(triple):
 
   return handler
 
 ##### Generator of iSel CHECK lines
 
 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)
+  check_label_format = '{} %s-LABEL: %s%s%s%s'.format(comment_marker)
   return common.add_checks(output_lines, comment_marker, prefix_list, func_dict,
-                           func_name, check_label_format, True, False,
+                           func_name, check_label_format, True, False, 1,
                            global_vars_seen_dict, is_filtered=is_filtered)

llvm/utils/update_cc_test_checks.py

if include_generated_funcs:
 
       # Now generate all the checks.
       def check_generator(my_output_lines, prefixes, func):
         if '-emit-llvm' in clang_args:
           return common.add_ir_checks(my_output_lines, '//',
                                       prefixes,
                                       func_dict, func, False,
                                       ti.args.function_signature,
+                                      ti.args.version,
                                       global_vars_seen_dict,
                                       is_filtered=builder.is_filtered())
         else:
           return asm.add_checks(my_output_lines, '//',
                                 prefixes,
                                 func_dict, func, global_vars_seen_dict,
                                 is_filtered=builder.is_filtered())
 
else:
                   common.add_ir_checks(
                       output_lines,
                       '//',
                       filecheck_run_list,
                       func_dict,
                       mangled,
                       False,
                       args.function_signature,
+                      args.version,
                       global_vars_seen_dict,
                       is_filtered=builder.is_filtered()))
               if line.rstrip('\n') == '//':
                 include_line = False
 
         if include_line:
           output_lines.append(line.rstrip('\n'))
 

llvm/utils/update_test_checks.py

if include_generated_funcs:
               lambda my_output_lines, prefixes, func: common.add_ir_checks(
                   my_output_lines,
                   ';',
                   prefixes,
                   func_dict,
                   func,
                   False,
                   args.function_signature,
+                  args.version,
                   global_vars_seen_dict,
                   is_filtered=builder.is_filtered())))
     else:
       # "Normal" mode.
       for input_line_info in ti.iterlines(output_lines):
         input_line = input_line_info.line
         args = input_line_info.args
         if is_in_function_start:
else:
               common.add_ir_checks(
                   output_lines,
                   ';',
                   prefix_list,
                   func_dict,
                   func_name,
                   args.preserve_names,
                   args.function_signature,
+                  args.version,
                   global_vars_seen_dict,
                   is_filtered=builder.is_filtered()))
           is_in_function_start = False
 
         m = common.IR_FUNCTION_RE.match(input_line)
         if m and not has_checked_pre_function_globals:
           if args.check_globals:
             generated_prefixes.extend(