[UpdateTestChecks] Warn if any function conflicts under the same prefix

[qiucf]

Summary

Currently the update test script will only output warning if all functions have conflict under different run lines with same prefix, and be silent if part of them conflict. This looks counterintuitive and leads to confusion sometimes.

[mtrofin]

lgtm code-wise. Added folks who were also interested in this, from a user perspective (I'm not a user of the tool, can't comment on that aspect.)

[mtrofin]

llvm/utils/UpdateTestChecks/common.py

339

Nit: may be worth renaming this function to more accurately reflect its new behavior. For example: _get_prefixes_failing_functions (or better name).

[pengfei]

llvm/utils/UpdateTestChecks/common.py

275

I think the warn only conflicting for all functions is used for the multi prefixes case, e.g. --check-prefixes=A,B;--check-prefixes=A,C when we hope the conflicted functions use B and C respectively. Under which case, warn A conflicted is annoying.

[lkail]

This patch looks worthy pursuing. We have some tests under test/CodeGen/PowerPC uses -check-prefixes=A,B,C and got conflicts when updating them without any warning and some parts of checks disappear.

llvm/utils/UpdateTestChecks/common.py

275

Agree.

[MaskRay]

llvm/test/tools/UpdateTestChecks/update_test_checks/prefix_conflicts.test

2

Add a period.

3

no need for -f. cp is sufficient.

llvm/utils/UpdateTestChecks/common.py

276

https://llvm.org/docs/CodingStandards.html#error-and-warning-messages

It is more common not to capitalize the message.

348

2 column indentation.

[MaskRay]

Looks reasonable but the test needs update. Request changes to remove this from folks' review queues

llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/conflict.ll

9

better to add another function

[qiucf]

Address some comments.

[lebedev.ri]

This should only warn if said occurrence would lead to the loss of the checks for the function.
I.e. it should not warn on prefix ALL given

; RUN: opt < %s -instcombine -S | FileCheck --check-prefixes=ALL,IC %s
; RUN: opt < %s -vectorize-slp -S | FileCheck --check-prefixes=ALL,SLP %s

[lebedev.ri]

This review seems to be stuck/dead, consider abandoning if no longer relevant.

[mtrofin]

D124306 should address this more comprehensively.

llvm/test/tools/UpdateTestChecks/update_llc_test_checks/prefix-never-matches.test

 # REQUIRES: x86-registered-target
 
 # RUN: cp -f %S/Inputs/prefix-never-matches.ll %t.ll
 # RUN: %update_llc_test_checks %t.ll 2>&1 | FileCheck %s
 # RUN: FileCheck --input-file=%t.ll %s --check-prefix=OUTPUT
 
-# CHECK: WARNING: Prefix A had conflicting output
-# OUTPUT-NOT: A:
+# CHECK: WARNING: prefix A had conflicting output
+# OUTPUT-NOT: A:
 

llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/conflict.ll

+; RUN: opt < %s -instcombine -S | FileCheck %s
+; RUN: opt < %s -vectorize-slp -S | FileCheck %s
+
+define i32 @diff1(i32 %X, i32 %Y) {
+  %Z = sub i32 %X, %Y
+  %Q = add i32 %Z, %Y
+  ret i32 %Q
+}
+
+define <2 x i32> @diff2(<2 x i32> %X) {
+  %A = and <2 x i32> %X, <i32 4, i32 4>
+  %B = lshr <2 x i32> %A, <i32 2, i32 2>
+  %C = icmp eq <2 x i32> %B, zeroinitializer
+  %D = zext <2 x i1> %C to <2 x i32>
+  ret <2 x i32> %D
+}
+
+define <4 x float> @same(<4 x float> %X, <4 x float> %Y) {
+  %A = shufflevector <4 x float> %X, <4 x float> %Y, <4 x i32> <i32 0, i32 2, i32 4, i32 6>
+  ret <4 x float> %A
+}

llvm/test/tools/UpdateTestChecks/update_test_checks/prefix-never-matches.test

 # RUN: cp -f %S/Inputs/prefix-never-matches.ll %t.ll
 # RUN: %update_test_checks %t.ll 2>&1 | FileCheck %s
 # RUN: FileCheck --input-file=%t.ll %s --check-prefix=OUTPUT
 
-# CHECK: WARNING: Prefix A had conflicting output
-# OUTPUT-NOT: A:
+# CHECK: WARNING: prefix A had conflicting output
+# OUTPUT-NOT: A:
 

llvm/test/tools/UpdateTestChecks/update_test_checks/prefix_conflicts.test

+# Verify the script warns about conflicts of prefix in multiple run lines.
+# RUN: cp %S/Inputs/conflict.ll %t.ll
+# RUN: %update_test_checks %t.ll 2>&1 | FileCheck %s
+# CHECK: prefix CHECK had conflicting output from different RUN lines for functions: diff1, diff2

llvm/utils/UpdateTestChecks/common.py

def __init__(self, string, extra, args_and_sig, attrs):
     self.scrub = string
     self.extrascrub = extra
     self.args_and_sig = args_and_sig
     self.attrs = attrs
   def is_same_except_arg_names(self, extrascrub, args_and_sig, attrs):
     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.attrs != attrs:
def __init__(self, run_list, flags, scrubber_args, path):
     self._global_var_dict = {}
     for tuple in run_list:
       for prefix in tuple[0]:
         self._func_dict.update({prefix:dict()})
         self._func_order.update({prefix: []})
         self._global_var_dict.update({prefix:dict()})
 
   def finish_and_get_func_dict(self):
-    for prefix in self._get_failed_prefixes():
-      warn('Prefix %s had conflicting output from different RUN lines for all functions in test %s' % (prefix,self._path,))
+    for prefix, funcs in self._get_failed_prefixes_with_functions():
+      warn('prefix %s had conflicting output from different RUN lines for functions: %s' % (
+          prefix, ', '.join(funcs)))
     return self._func_dict
 
   def func_order(self):
     return self._func_order
 
   def global_var_dict(self):
     return self._global_var_dict
 
   def process_run_line(self, function_re, scrubber, raw_tool_output, prefixes):
     build_global_values_dictionary(self._global_var_dict, raw_tool_output, prefixes)
     for m in function_re.finditer(raw_tool_output):
       if not m:
         continue
       func = m.group('func')
       body = m.group('body')
       attrs = m.group('attrs') if self._check_attributes 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 = ''
       scrubbed_body = do_scrub(body, scrubber, self._scrubber_args,
                                extra=False)
       scrubbed_extra = do_scrub(body, scrubber, self._scrubber_args,
                                 extra=True)
       if 'analysis' in m.groupdict():
         analysis = m.group('analysis')
         if analysis.lower() != 'cost model analysis':
           warn('Unsupported analysis mode: %r!' % (analysis,))
       if func.startswith('stress'):
         # We only use the last line of the function body for stress tests.
         scrubbed_body = '\n'.join(scrubbed_body.splitlines()[-1:])
       if self._verbose:
for m in function_re.finditer(raw_tool_output):
               # indicate that.
               self._func_dict[prefix][func] = None
               continue
 
         self._func_dict[prefix][func] = function_body(
             scrubbed_body, scrubbed_extra, args_and_sig, attrs)
         self._func_order[prefix].append(func)
 
-  def _get_failed_prefixes(self):
-    # This returns the list of those prefixes that failed to match any function,
-    # because there were conflicting bodies produced by different RUN lines, in
-    # all instances of the prefix. Effectively, this prefix is unused and should
-    # be removed.
+  def _get_failed_prefixes_with_functions(self):
+    # This returns the list of prefixes that failed to match some functions,
+    # along with function names, because there were conflicting bodies produced
+    # by different RUN lines, in all instrances of the prefix. This prefix needs
+    # to be split or removed.
     for prefix in self._func_dict:
-      if (self._func_dict[prefix] and
-          (not [fct for fct in self._func_dict[prefix]
-                if self._func_dict[prefix][fct] is not None])):
-        yield prefix
-
+      if self._func_dict[prefix]:
+        funcs = [fct for fct in self._func_dict[prefix]
+                 if self._func_dict[prefix][fct] is None]
+        if funcs:
+          yield (prefix, funcs)
 
 ##### Generator of LLVM IR CHECK lines
 
 SCRUB_IR_COMMENT_RE = re.compile(r'\s*;.*')
 
 # TODO: We should also derive check lines for global, debug, loop declarations, etc..
 
 class NamelessValue: