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
YuriPlyakhin · Jul 20, 2022 · 5a4033c · 5a4033c
1 parent 8ba794b
commit 5a4033c
Show file tree

Hide file tree

Showing 11 changed files with 84 additions and 25 deletions.
diff --git a/clang/test/utils/update_cc_test_checks/Inputs/ifdef.c 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -23,10 +23,10 @@ class string:
     flags=(re.M | re.S))
 
 ASM_FUNCTION_ARM_RE = re.compile(
-    r'^(?P<func>[0-9a-zA-Z_]+):\n' # f: (name of function)
+    r'^(?P<func>[0-9a-zA-Z_$]+):\n' # f: (name of function)
     r'\s+\.fnstart\n' # .fnstart
-    r'(?P<body>.*?)\n' # (body of the function)
-    r'.Lfunc_end[0-9]+:', # .Lfunc_end0: or # -- End function
+    r'(?P<body>.*?)' # (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<func>[^ \t])[ \t]*@[ \t]--[ \t]Begin[ \t]function[ \t]"?(?P=func)"?'
+    r'@[ \t]--[ \t]Begin[ \t]function[ \t](?P<func>[^ \t]+?)\n'
+    r'^[ \t]*\.globl[ \t]*_(?P=func)[ \t]*'
     r'(?P<directives>.*?)'
     r'^_(?P=func):\n[ \t]*'
     r'(?P<body>.*?)'

diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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