[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

1

Add a period.

2

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

8

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_test_checks/Inputs/conflict.ll

+; RUN: opt < %s -instcombine -S | FileCheck %s
+; RUN: opt < %s -vectorize-slp -S | FileCheck %s
+
+define i32 @example(i32 %X, i32 %Y) {
+  %Z = sub i32 %X, %Y
+  %Q = add i32 %Z, %Y
+  ret i32 %Q
+}

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

+# Verify the script warns about conflicts of prefix in multiple run lines
+# RUN: cp -f %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: example

llvm/utils/UpdateTestChecks/common.py

     self._func_dict = {}
     self._func_order = {}
     for tuple in run_list:
       for prefix in tuple[0]:
         self._func_dict.update({prefix:dict()})
         self._func_order.update({prefix: []})
 
   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' % (prefix,))
+    for prefix, funcs in self._get_failed_prefixes():
+      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 process_run_line(self, function_re, scrubber, raw_tool_output, prefixes):
     for m in function_re.finditer(raw_tool_output):
       if not m:

               # 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,
+    # This returns the list of those prefixes that failed to match some functions,
     # 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.
+    # all instances 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..