asb created this revision.
asb added reviewers: arichardson, nikic, mtrofin.
Herald added subscribers: wingo, pmatos, StephenFan, luismarques,
sameer.abuasal, s.egerton, PkmX, simoncook.
Herald added a project: All.
asb requested review of this revision.
Herald added projects: clang, LLVM.
Herald added a subscriber: cfe-commits.
This is an alternative, less intrusive take on D133943
<https://reviews.llvm.org/D133943> (where we were unable to find consensus on
how to handle updating existing in-tree tests).
Zooming right out, the basic problem is that `--function-signature` checks
arguments but now the return type. This limitation means it's not very useful
for my use case (making RISC-V ABI tests more maintainable). The value of this
is huge - manually updating CHECK lines in the previous ABI tests was a massive
timesink, and with this feature it is _massively_ easier.
Although it would be cleaner in a sense to update `--function-signature` and
find agreement on transitioning in-tree tests to the new behaviour, I think
adding the separate command-line flag is the best option available at this
point. It's hopefully much easier to get this reviewed and landed, thus
unblocking the ABI test refactorings and follow-on bug fixes that rely on it.
There's also a clear migration path - in the future `--function-signature`
could be updated to act like `--full-function-signature` and
`--full-function-signature` be either removed or left as an alias. It would be
slightly better to support `--function-signature=full`, but I couldn't find a
way to accept an optional argument but only with `--foo=bar` syntax (rather
than `--foo bar`, which could break existing command lines) with default
argparse routines.
I'd really, really, really appreciate help on unblocking this (reviews or other
feedback). Thank you in advance.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D142373
Files:
clang/test/utils/update_cc_test_checks/Inputs/mangled_names.c.fullfuncsig.expected
clang/test/utils/update_cc_test_checks/mangled_names.test
llvm/utils/UpdateTestChecks/asm.py
llvm/utils/UpdateTestChecks/common.py
llvm/utils/UpdateTestChecks/isel.py
llvm/utils/update_cc_test_checks.py
llvm/utils/update_test_checks.py
Index: llvm/utils/update_test_checks.py
===================================================================
--- llvm/utils/update_test_checks.py
+++ llvm/utils/update_test_checks.py
@@ -181,6 +181,7 @@
func,
False,
args.function_signature,
+ False,
global_vars_seen_dict,
is_filtered=builder.is_filtered())))
else:
@@ -207,6 +208,7 @@
func_name,
args.preserve_names,
args.function_signature,
+ False,
global_vars_seen_dict,
is_filtered=builder.is_filtered()))
is_in_function_start = False
Index: llvm/utils/update_cc_test_checks.py
===================================================================
--- llvm/utils/update_cc_test_checks.py
+++ llvm/utils/update_cc_test_checks.py
@@ -138,6 +138,8 @@
args.opt = 'opt'
else:
args.opt = os.path.join(args.llvm_bin, 'opt')
+ if args.full_function_signature and not args.function_signature:
+ args.function_signature = True
def find_executable(executable):
@@ -167,6 +169,8 @@
help='Use more regex for x86 matching to reduce diffs between various subtargets')
parser.add_argument('--function-signature', action='store_true',
help='Keep function signature information around for the check line')
+ parser.add_argument('--full-function-signature', action='store_true',
+ help='Keep function signature information (including return type) around for the check line')
parser.add_argument('--check-attributes', action='store_true',
help='Check "Function Attributes" for functions')
parser.add_argument('--check-globals', action='store_true',
@@ -353,6 +357,7 @@
prefixes,
func_dict, func, False,
ti.args.function_signature,
+ ti.args.full_function_signature,
global_vars_seen_dict,
is_filtered=builder.is_filtered())
else:
@@ -420,6 +425,7 @@
mangled,
False,
args.function_signature,
+ args.full_function_signature,
global_vars_seen_dict,
is_filtered=builder.is_filtered()))
if line.rstrip('\n') == '//':
Index: llvm/utils/UpdateTestChecks/isel.py
===================================================================
--- llvm/utils/UpdateTestChecks/isel.py
+++ llvm/utils/UpdateTestChecks/isel.py
@@ -51,7 +51,7 @@
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,
- global_vars_seen_dict, is_filtered=is_filtered)
+ False, global_vars_seen_dict, is_filtered=is_filtered)
Index: llvm/utils/UpdateTestChecks/common.py
===================================================================
--- llvm/utils/UpdateTestChecks/common.py
+++ llvm/utils/UpdateTestChecks/common.py
@@ -329,7 +329,7 @@
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))
@@ -442,13 +442,14 @@
# 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))
@@ -461,6 +462,8 @@
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)
@@ -536,6 +539,7 @@
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():
@@ -590,9 +594,11 @@
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):
@@ -607,8 +613,8 @@
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
@@ -893,7 +899,7 @@
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, check_funcdef_attrs_and_ret, 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 = []
@@ -941,6 +947,11 @@
printed_prefixes.append(checkprefix)
attrs = str(func_dict[checkprefix][func_name].attrs)
attrs = '' if attrs == 'None' else attrs
+ if check_funcdef_attrs_and_ret:
+ 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)
@@ -948,10 +959,10 @@
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.
@@ -1027,20 +1038,28 @@
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, full_function_sig,
global_vars_seen_dict, is_filtered):
+ function_sig = function_sig or full_function_sig
# Label format is based on IR string.
- function_def_regex = 'define {{[^@]+}}' if function_sig else ''
+ if full_function_sig:
+ 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,
+ full_function_sig, 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, False, global_vars_seen_dict,
is_filtered)
def build_global_values_dictionary(glob_val_dict, raw_tool_output, prefixes):
Index: llvm/utils/UpdateTestChecks/asm.py
===================================================================
--- llvm/utils/UpdateTestChecks/asm.py
+++ llvm/utils/UpdateTestChecks/asm.py
@@ -534,7 +534,7 @@
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, False,
global_vars_seen_dict, is_filtered=is_filtered)
Index: clang/test/utils/update_cc_test_checks/mangled_names.test
===================================================================
--- clang/test/utils/update_cc_test_checks/mangled_names.test
+++ clang/test/utils/update_cc_test_checks/mangled_names.test
@@ -16,3 +16,6 @@
# 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 the --full-function-signature flag
+# RUN: %update_cc_test_checks %t.c --full-function-signature
+# RUN: diff -u %t.c %S/Inputs/mangled_names.c.fullfuncsig.expected
Index: clang/test/utils/update_cc_test_checks/Inputs/mangled_names.c.fullfuncsig.expected
===================================================================
--- /dev/null
+++ clang/test/utils/update_cc_test_checks/Inputs/mangled_names.c.fullfuncsig.expected
@@ -0,0 +1,43 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --function-signature --full-function-signature
+// Example input for update_cc_test_checks
+// RUN: %clang_cc1 -no-opaque-pointers -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]], i64* [[A_ADDR]], align 8
+// CHECK-NEXT: store i32 [[B]], i32* [[B_ADDR]], align 4
+// CHECK-NEXT: [[TMP0:%.*]] = load i64, i64* [[A_ADDR]], align 8
+// CHECK-NEXT: [[TMP1:%.*]] = load i32, i32* [[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]], i64* [[A_ADDR]], align 8
+// CHECK-NEXT: store i32 [[B]], i32* [[B_ADDR]], align 4
+// CHECK-NEXT: store i32 [[C]], i32* [[C_ADDR]], align 4
+// CHECK-NEXT: [[TMP0:%.*]] = load i64, i64* [[A_ADDR]], align 8
+// CHECK-NEXT: [[TMP1:%.*]] = load i32, i32* [[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, i32* [[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;
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
