From 5a4033c36716de0cee75eb28b95cce44ae239cd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolai=20H=C3=A4hnle?= Date: Tue, 19 Jul 2022 09:43:58 +0200 Subject: [PATCH] update-test-checks: safely handle tests with #if's There is at least one Clang test (clang/test/CodeGen/arm_acle.c) which has functions guarded by #if's that cause those functions to be compiled only for a subset of RUN lines. This results in a case where one RUN line has a body for the function and another doesn't. Treat this case as a conflict for any prefixes that the two RUN lines have in common. This change exposed a bug where functions with '$' in the name weren't properly recognized in ARM assembly (despite there being a test case that was supposed to catch the problem!). This bug is fixed as well. Differential Revision: https://reviews.llvm.org/D130089 --- .../update_cc_test_checks/Inputs/ifdef.c | 12 ++++++ .../Inputs/ifdef.c.expected | 21 ++++++++++ .../utils/update_cc_test_checks/ifdef.test | 8 ++++ .../Inputs/arm_function_name.ll | 7 ++-- .../Inputs/arm_function_name.ll.expected | 7 ++-- llvm/utils/UpdateTestChecks/asm.py | 9 +++-- llvm/utils/UpdateTestChecks/common.py | 40 +++++++++++++------ llvm/utils/update_analyze_test_checks.py | 2 + llvm/utils/update_cc_test_checks.py | 1 + llvm/utils/update_llc_test_checks.py | 1 + llvm/utils/update_test_checks.py | 1 + 11 files changed, 84 insertions(+), 25 deletions(-) create mode 100644 clang/test/utils/update_cc_test_checks/Inputs/ifdef.c create mode 100644 clang/test/utils/update_cc_test_checks/Inputs/ifdef.c.expected create mode 100644 clang/test/utils/update_cc_test_checks/ifdef.test diff --git a/clang/test/utils/update_cc_test_checks/Inputs/ifdef.c b/clang/test/utils/update_cc_test_checks/Inputs/ifdef.c new file mode 100644 index 0000000000000..79d73a2de4863 --- /dev/null +++ b/clang/test/utils/update_cc_test_checks/Inputs/ifdef.c @@ -0,0 +1,12 @@ +// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK %s +// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -DFOO | FileCheck -check-prefixes=CHECK,FOO %s + +#ifdef FOO +int foo() { + return 1; +} +#endif + +int bar() { + return 2; +} diff --git a/clang/test/utils/update_cc_test_checks/Inputs/ifdef.c.expected b/clang/test/utils/update_cc_test_checks/Inputs/ifdef.c.expected new file mode 100644 index 0000000000000..c1fb72db9339e --- /dev/null +++ b/clang/test/utils/update_cc_test_checks/Inputs/ifdef.c.expected @@ -0,0 +1,21 @@ +// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py +// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK %s +// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -DFOO | FileCheck -check-prefixes=CHECK,FOO %s + +#ifdef FOO +// FOO-LABEL: @foo( +// FOO-NEXT: entry: +// FOO-NEXT: ret i32 1 +// +int foo() { + return 1; +} +#endif + +// CHECK-LABEL: @bar( +// CHECK-NEXT: entry: +// CHECK-NEXT: ret i32 2 +// +int bar() { + return 2; +} diff --git a/clang/test/utils/update_cc_test_checks/ifdef.test b/clang/test/utils/update_cc_test_checks/ifdef.test new file mode 100644 index 0000000000000..6e4c6319a31c9 --- /dev/null +++ b/clang/test/utils/update_cc_test_checks/ifdef.test @@ -0,0 +1,8 @@ +## Test that functions that are only compiled in a subset of RUN lines are +## handled correctly + +# RUN: cp %S/Inputs/ifdef.c %t.c && %update_cc_test_checks %t.c +# RUN: diff -u %S/Inputs/ifdef.c.expected %t.c +## Check that re-running update_cc_test_checks doesn't change the output +# RUN: %update_cc_test_checks %t.c +# RUN: diff -u %S/Inputs/ifdef.c.expected %t.c diff --git a/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll b/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll index fa03d8cc51cd9..5f3edb6edf478 100644 --- a/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll +++ b/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll @@ -1,9 +1,8 @@ ; Check that we accept functions with '$' in the name. -; TODO: This is not handled correcly on 32bit ARM and needs to be fixed. -; RUN: llc -mtriple=armv7-unknown-linux < %s | FileCheck --prefix=LINUX %s -; RUN: llc -mtriple=armv7-apple-darwin < %s | FileCheck --prefix=DARWIN %s -; RUN: llc -mtriple=armv7-apple-ios < %s | FileCheck --prefix=IOS %s +; RUN: llc -mtriple=armv7-unknown-linux < %s | FileCheck %s +; RUN: llc -mtriple=armv7-apple-darwin < %s | FileCheck %s +; RUN: llc -mtriple=armv7-apple-ios < %s | FileCheck %s define hidden i32 @"_Z54bar$ompvariant$bar"() { entry: diff --git a/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll.expected b/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll.expected index e15d788ce34f4..6a8684f85575a 100644 --- a/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll.expected +++ b/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll.expected @@ -1,10 +1,9 @@ ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py ; Check that we accept functions with '$' in the name. -; TODO: This is not handled correcly on 32bit ARM and needs to be fixed. -; RUN: llc -mtriple=armv7-unknown-linux < %s | FileCheck --prefix=LINUX %s -; RUN: llc -mtriple=armv7-apple-darwin < %s | FileCheck --prefix=DARWIN %s -; RUN: llc -mtriple=armv7-apple-ios < %s | FileCheck --prefix=IOS %s +; RUN: llc -mtriple=armv7-unknown-linux < %s | FileCheck %s +; RUN: llc -mtriple=armv7-apple-darwin < %s | FileCheck %s +; RUN: llc -mtriple=armv7-apple-ios < %s | FileCheck %s define hidden i32 @"_Z54bar$ompvariant$bar"() { ; CHECK-LABEL: _Z54bar$ompvariant$bar: diff --git a/llvm/utils/UpdateTestChecks/asm.py b/llvm/utils/UpdateTestChecks/asm.py index 953db74f3d840..821b2d0019075 100644 --- a/llvm/utils/UpdateTestChecks/asm.py +++ b/llvm/utils/UpdateTestChecks/asm.py @@ -23,10 +23,10 @@ class string: flags=(re.M | re.S)) ASM_FUNCTION_ARM_RE = re.compile( - r'^(?P[0-9a-zA-Z_]+):\n' # f: (name of function) + r'^(?P[0-9a-zA-Z_$]+):\n' # f: (name of function) r'\s+\.fnstart\n' # .fnstart - r'(?P.*?)\n' # (body of the function) - r'.Lfunc_end[0-9]+:', # .Lfunc_end0: or # -- End function + r'(?P.*?)' # (body of the function) + r'^.Lfunc_end[0-9]+:', # .Lfunc_end0: or # -- End function flags=(re.M | re.S)) ASM_FUNCTION_AARCH64_RE = re.compile( @@ -128,7 +128,8 @@ class string: flags=(re.M | re.S)) ASM_FUNCTION_ARM_DARWIN_RE = re.compile( - r'^[ \t]*\.globl[ \t]*_(?P[^ \t])[ \t]*@[ \t]--[ \t]Begin[ \t]function[ \t]"?(?P=func)"?' + r'@[ \t]--[ \t]Begin[ \t]function[ \t](?P[^ \t]+?)\n' + r'^[ \t]*\.globl[ \t]*_(?P=func)[ \t]*' r'(?P.*?)' r'^_(?P=func):\n[ \t]*' r'(?P.*?)' diff --git a/llvm/utils/UpdateTestChecks/common.py b/llvm/utils/UpdateTestChecks/common.py index a1e883d8ea7dd..8e02488da985e 100644 --- a/llvm/utils/UpdateTestChecks/common.py +++ b/llvm/utils/UpdateTestChecks/common.py @@ -496,6 +496,7 @@ def __init__(self, run_list, flags, scrubber_args, path): self._func_dict = {} self._func_order = {} self._global_var_dict = {} + self._processed_prefixes = set() for tuple in run_list: for prefix in tuple[0]: self._func_dict.update({prefix:dict()}) @@ -584,30 +585,43 @@ def process_run_line(self, function_re, scrubber, raw_tool_output, prefixes, is_ scrubbed_body) if func in self._func_dict[prefix]: - if (self._func_dict[prefix][func] is None or - 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): - if (self._func_dict[prefix][func] is not None and - self._func_dict[prefix][func].is_same_except_arg_names( + 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)): + if self._func_dict[prefix][func].is_same_except_arg_names( scrubbed_extra, args_and_sig, attrs, - is_backend)): + is_backend): self._func_dict[prefix][func].scrub = scrubbed_extra self._func_dict[prefix][func].args_and_sig = args_and_sig - continue 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 accross RUN lines. We use None to # indicate that. self._func_dict[prefix][func] = None - continue - - self._func_dict[prefix][func] = function_body( - scrubbed_body, scrubbed_extra, args_and_sig, attrs, func_name_separator) - self._func_order[prefix].append(func) + 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) + 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 + + def processed_prefixes(self, prefixes): + """ + Mark a set of prefixes as having had at least one applicable RUN line fully + processed. This is used to filter out function bodies that don't have + outputs for all RUN lines. + """ + self._processed_prefixes.update(prefixes) def get_failed_prefixes(self): # This returns the list of those prefixes that failed to match any function, diff --git a/llvm/utils/update_analyze_test_checks.py b/llvm/utils/update_analyze_test_checks.py index c48e2db272833..168d358bc3d83 100755 --- a/llvm/utils/update_analyze_test_checks.py +++ b/llvm/utils/update_analyze_test_checks.py @@ -124,6 +124,8 @@ def main(): common.warn('Don\'t know how to deal with this output') continue + builder.processed_prefixes(prefixes) + func_dict = builder.finish_and_get_func_dict() is_in_function = False is_in_function_start = False diff --git a/llvm/utils/update_cc_test_checks.py b/llvm/utils/update_cc_test_checks.py index 211892a4d682e..b9e91f19461b0 100755 --- a/llvm/utils/update_cc_test_checks.py +++ b/llvm/utils/update_cc_test_checks.py @@ -214,6 +214,7 @@ def get_function_body(builder, args, filename, clang_args, extra_commands, builder.process_run_line( common.OPT_FUNCTION_RE, common.scrub_body, raw_tool_output, prefixes, False) + builder.processed_prefixes(prefixes) else: print('The clang command line should include -emit-llvm as asm tests ' 'are discouraged in Clang testsuite.', file=sys.stderr) diff --git a/llvm/utils/update_llc_test_checks.py b/llvm/utils/update_llc_test_checks.py index 66eb5d4e4f5e3..4ce5534a2b735 100755 --- a/llvm/utils/update_llc_test_checks.py +++ b/llvm/utils/update_llc_test_checks.py @@ -137,6 +137,7 @@ def main(): scrubber, function_re = output_type.get_run_handler(triple) builder.process_run_line(function_re, scrubber, raw_tool_output, prefixes, True) + builder.processed_prefixes(prefixes) func_dict = builder.finish_and_get_func_dict() global_vars_seen_dict = {} diff --git a/llvm/utils/update_test_checks.py b/llvm/utils/update_test_checks.py index c6d8d26f00758..f16103b892b06 100755 --- a/llvm/utils/update_test_checks.py +++ b/llvm/utils/update_test_checks.py @@ -120,6 +120,7 @@ def main(): verbose=ti.args.verbose) builder.process_run_line(common.OPT_FUNCTION_RE, common.scrub_body, raw_tool_output, prefixes, False) + builder.processed_prefixes(prefixes) func_dict = builder.finish_and_get_func_dict() is_in_function = False