[llvm-mca] Stricter checking from update_mca_test_checks.py

[gbedwell]

Summary

If any prefixes have been specified on the RUN lines that do not end up
ever actually getting printed, raise an Error. This is either an
indication that the run lines just need cleaning up, or that something
is more fundamentally wrong with the test.

Also raise an Error if there are any blocks which cannot be checked
because they are not uniquely covered by a prefix.

Ran "py update_mca_test_checks.py ..\test\tools\llvm-mca\*\*.s ..\test\tools\llvm-mca\*\*\*.s" to regenerate all tests and fixed-up a few which the new errors found issues with.

[lebedev.ri]

I did not review this yet, but it is Nice!
The other utils really need to improve in this regard (PR37752)

[gbedwell]

utils/update_mca_test_checks.py

446

I spotted a minor indentation issue here. I'll fix before committing.

[lebedev.ri]

It's a bit hard to review, the patch misses context (-U99999).

[gbedwell]

In D48276#1135165, @lebedev.ri wrote:

It's a bit hard to review, the patch misses context (-U99999).

Good point. I lost the context when I was creating diffs for raising PR37834. I'll re-upload.

[gbedwell]

Forgot to add llvm-commits

[lebedev.ri]

I think this makes sense.
It's not really feasible to meaningfully review these utility scripts, since there are no tests,
but phabricator does not like unaccepted differentials, so other than a few minor nits, LG.

utils/update_mca_test_checks.py

382–384

I'm not sure, is this related to the rest of the diff?

492–496

Why update? The return isn't used.
Could you just do used_prefixes |= ?
Would be more readable, too.

[RKSimon]

@gbedwell What is the plan for this patch?

[gbedwell]

Rebased.

[gbedwell]

utils/update_mca_test_checks.py

382–384

This is part of the same stricter checking. It, for example, caught the issue in test/tools/llvm-mca/X86/option-all-stats-1.s where the following RUN commands were generating slightly different output, but without any differentiating check prefixes.

# RUN: llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 -all-stats=false < %s | FileCheck %s -check-prefix=ALL
# RUN: llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2                  < %s | FileCheck %s -check-prefix=ALL

[gbedwell]

I've chatted with @andreadb and @RKSimon offline a bit. It would be nice to try and avoid some of the obviously duplicated blocks in the tests. For example, the added blocks in option-all-stats-1.s above. That's not actually an issue related to this specific patch, and the generated tests are still valid, but sometimes look a bit messy.

I don't want to hold this patch up based on that, but on the other hand if I can come up with something quickly, it might make sense to land them at the same time to prevent too much churn. I'll update when I know more.

[gbedwell]

Rebase on top of D52560 which improves the block analysis significantly.

[andreadb]

Thanks Greg.

It looks good to me.

test/tools/llvm-mca/X86/option-all-views-2.s

 # NOTE: Assertions have been autogenerated by utils/update_mca_test_checks.py
 # RUN: llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 -all-views                          < %s | FileCheck %s -check-prefix=ALL -check-prefix=FULLREPORT
 # RUN: llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 -all-views -resource-pressure       < %s | FileCheck %s -check-prefix=ALL -check-prefix=FULLREPORT
 # RUN: llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 -resource-pressure -all-views       < %s | FileCheck %s -check-prefix=ALL -check-prefix=FULLREPORT
 # RUN: llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 -resource-pressure=false -all-views < %s | FileCheck %s -check-prefix=ALL -check-prefix=FULLREPORT
-# RUN: llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 -all-views -resource-pressure=false < %s | FileCheck %s -check-prefix=ALL -check-prefix=NORPV
+# RUN: llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 -all-views -resource-pressure=false < %s | FileCheck %s -check-prefix=ALL
 
 add %eax, %eax
 
 # ALL:             Iterations:        100
 # ALL-NEXT:        Instructions:      100
 # ALL-NEXT:        Total Cycles:      103
 # ALL-NEXT:        Total uOps:        100
 

test/tools/llvm-mca/X86/register-file-statistics.s

 # NOTE: Assertions have been autogenerated by utils/update_mca_test_checks.py
 # RUN: llvm-mca %s -mtriple=x86_64-unknown-unknown -mcpu=btver2 -iterations=1 -all-stats=false -all-views=false -register-file-stats < %s | FileCheck --check-prefixes=ALL,BTVER2 %s
 # RUN: llvm-mca %s -mtriple=x86_64-unknown-unknown -mcpu=znver1 -iterations=1 -all-stats=false -all-views=false -register-file-stats < %s | FileCheck --check-prefixes=ALL,ZNVER1 %s
-# RUN: llvm-mca %s -mtriple=x86_64-unknown-unknown -mcpu=sandybridge -iterations=1 -all-stats=false -all-views=false -register-file-stats < %s | FileCheck --check-prefixes=ALL,SNB %s
-# RUN: llvm-mca %s -mtriple=x86_64-unknown-unknown -mcpu=ivybridge -iterations=1 -all-stats=false -all-views=false -register-file-stats < %s | FileCheck --check-prefixes=ALL,IVB %s
-# RUN: llvm-mca %s -mtriple=x86_64-unknown-unknown -mcpu=haswell -iterations=1 -all-stats=false -all-views=false -register-file-stats < %s | FileCheck --check-prefixes=ALL,HSW %s
-# RUN: llvm-mca %s -mtriple=x86_64-unknown-unknown -mcpu=broadwell -iterations=1 -all-stats=false -all-views=false -register-file-stats < %s | FileCheck --check-prefixes=ALL,BDW %s
-# RUN: llvm-mca %s -mtriple=x86_64-unknown-unknown -mcpu=knl -iterations=1 -all-stats=false -all-views=false -register-file-stats < %s | FileCheck --check-prefixes=ALL,KNL %s
-# RUN: llvm-mca %s -mtriple=x86_64-unknown-unknown -mcpu=skylake -iterations=1 -all-stats=false -all-views=false -register-file-stats < %s | FileCheck --check-prefixes=ALL,SKX %s
-# RUN: llvm-mca %s -mtriple=x86_64-unknown-unknown -mcpu=skylake-avx512 -iterations=1 -all-stats=false -all-views=false -register-file-stats < %s | FileCheck --check-prefixes=ALL,SKX-AVX512 %s
-# RUN: llvm-mca %s -mtriple=x86_64-unknown-unknown -mcpu=slm -iterations=1 -all-stats=false -all-views=false -register-file-stats < %s | FileCheck --check-prefixes=ALL,SLM %s
+# RUN: llvm-mca %s -mtriple=x86_64-unknown-unknown -mcpu=sandybridge -iterations=1 -all-stats=false -all-views=false -register-file-stats < %s | FileCheck --check-prefixes=ALL %s
+# RUN: llvm-mca %s -mtriple=x86_64-unknown-unknown -mcpu=ivybridge -iterations=1 -all-stats=false -all-views=false -register-file-stats < %s | FileCheck --check-prefixes=ALL %s
+# RUN: llvm-mca %s -mtriple=x86_64-unknown-unknown -mcpu=haswell -iterations=1 -all-stats=false -all-views=false -register-file-stats < %s | FileCheck --check-prefixes=ALL %s
+# RUN: llvm-mca %s -mtriple=x86_64-unknown-unknown -mcpu=broadwell -iterations=1 -all-stats=false -all-views=false -register-file-stats < %s | FileCheck --check-prefixes=ALL %s
+# RUN: llvm-mca %s -mtriple=x86_64-unknown-unknown -mcpu=knl -iterations=1 -all-stats=false -all-views=false -register-file-stats < %s | FileCheck --check-prefixes=ALL %s
+# RUN: llvm-mca %s -mtriple=x86_64-unknown-unknown -mcpu=skylake -iterations=1 -all-stats=false -all-views=false -register-file-stats < %s | FileCheck --check-prefixes=ALL %s
+# RUN: llvm-mca %s -mtriple=x86_64-unknown-unknown -mcpu=skylake-avx512 -iterations=1 -all-stats=false -all-views=false -register-file-stats < %s | FileCheck --check-prefixes=ALL %s
+# RUN: llvm-mca %s -mtriple=x86_64-unknown-unknown -mcpu=slm -iterations=1 -all-stats=false -all-views=false -register-file-stats < %s | FileCheck --check-prefixes=ALL %s
 
 xor %eax, %ebx
 
-# ALL:             Register File statistics:
-# ALL-NEXT:        Total number of mappings created:    2
-# ALL-NEXT:        Max number of mappings used:         2
+# ALL:         Register File statistics:
+# ALL-NEXT:    Total number of mappings created:    2
+# ALL-NEXT:    Max number of mappings used:         2
 
-# BTVER2:          *  Register File #1 -- JFpuPRF:
-# BTVER2-NEXT:        Number of physical registers:     72
-# BTVER2-NEXT:        Total number of mappings created: 0
-# BTVER2-NEXT:        Max number of mappings used:      0
+# BTVER2:      *  Register File #1 -- JFpuPRF:
+# BTVER2-NEXT:    Number of physical registers:     72
+# BTVER2-NEXT:    Total number of mappings created: 0
+# BTVER2-NEXT:    Max number of mappings used:      0
 
-# ZNVER1:          *  Register File #1 -- ZnFpuPRF:
-# ZNVER1-NEXT:        Number of physical registers:     160
-# ZNVER1-NEXT:        Total number of mappings created: 0
-# ZNVER1-NEXT:        Max number of mappings used:      0
+# ZNVER1:      *  Register File #1 -- ZnFpuPRF:
+# ZNVER1-NEXT:    Number of physical registers:     160
+# ZNVER1-NEXT:    Total number of mappings created: 0
+# ZNVER1-NEXT:    Max number of mappings used:      0
 
-# BTVER2:          *  Register File #2 -- JIntegerPRF:
-# BTVER2-NEXT:        Number of physical registers:     64
-# BTVER2-NEXT:        Total number of mappings created: 2
-# BTVER2-NEXT:        Max number of mappings used:      2
+# BTVER2:      *  Register File #2 -- JIntegerPRF:
+# BTVER2-NEXT:    Number of physical registers:     64
+# BTVER2-NEXT:    Total number of mappings created: 2
+# BTVER2-NEXT:    Max number of mappings used:      2
 
-# ZNVER1:          *  Register File #2 -- ZnIntegerPRF:
-# ZNVER1-NEXT:        Number of physical registers:     168
-# ZNVER1-NEXT:        Total number of mappings created: 2
-# ZNVER1-NEXT:        Max number of mappings used:      2
+# ZNVER1:      *  Register File #2 -- ZnIntegerPRF:
+# ZNVER1-NEXT:    Number of physical registers:     168
+# ZNVER1-NEXT:    Total number of mappings created: 2
+# ZNVER1-NEXT:    Max number of mappings used:      2

utils/update_mca_test_checks.py

for i, block_text in enumerate(block_infos[block_num]):
       for s in pruned_sets[i]:
         s = sorted(list(s))
         if s:
           current_set.add(s[0])
           if len(s) > 1:
             _warn('Multiple prefixes generating same output: {} '
                   '(discarding {})'.format(','.join(s), ','.join(s[1:])))
 
+      if block_text and not current_set:
+        raise Error(
+          'block not captured by existing prefixes:\n\n{}'.format(block_text))
       block_infos[block_num][block_text] = sorted(list(current_set))
 
     # If we have multiple block_texts, try to break them down further to avoid
     # the case where we have very similar block_texts repeated after each
     # other.
     if common_prefix and len(block_infos[block_num]) > 1:
       # We'll only attempt this if each of the block_texts have the same number
       # of lines as each other.
       same_num_Lines = (len(set(len(k.splitlines())
                                 for k in block_infos[block_num].keys())) == 1)
       if same_num_Lines:
         breakdown = _break_down_block(block_infos[block_num], common_prefix)
         if breakdown:
           block_infos[block_num] = breakdown
 
   return block_infos
 
 
 def _write_block(output, block, not_prefix_set, common_prefix, prefix_pad):
   """ Write an individual block, with correct padding on the prefixes.
+      Returns a set of all of the prefixes that it has written.
   """
   end_prefix = ':     '
   previous_prefix = None
   num_lines_of_prefix = 0
+  written_prefixes = set()
 
   for prefix, line in block:
     if prefix in not_prefix_set:
       _warn('not writing for prefix {0} due to presence of "{0}-NOT:" '
             'in input file.'.format(prefix))
       continue
 
     # If the previous line isn't already blank and we're writing more than one
     # line for the current prefix output a blank line first, unless either the
     # current of previous prefix is common to all.
     num_lines_of_prefix += 1
     if prefix != previous_prefix:
       if output and output[-1]:
         if num_lines_of_prefix > 1 or any(p == common_prefix
                                           for p in (prefix, previous_prefix)):
           output.append('')
       num_lines_of_prefix = 0
       previous_prefix = prefix
 
+    written_prefixes.add(prefix)
     output.append(
         '{} {}{}{} {}'.format(COMMENT_CHAR,
                               prefix,
                               end_prefix,
                               ' ' * (prefix_pad - len(prefix)),
                               line).rstrip())
     end_prefix = '-NEXT:'
 
   output.append('')
+  return written_prefixes
 
 
 def _write_output(test_path, input_lines, prefix_list, block_infos,  # noqa
                   args, common_prefix, prefix_pad):
   prefix_set = set([prefix for prefixes, _ in prefix_list
                     for prefix in prefixes])
   not_prefix_set = set()
 
   output_lines = []
   for input_line in input_lines:
     if input_line.startswith(ADVERT_PREFIX):
       continue
 
     if input_line.startswith(COMMENT_CHAR):
for input_line in input_lines:
     else:
       continue
 
   # Add a blank line before the new checks if required.
   if len(output_lines) > 0 and output_lines[-1]:
     output_lines.append('')
 
   output_check_lines = []
+  used_prefixes = set()
   for block_num in range(len(block_infos)):
     for block_text in sorted(block_infos[block_num]):
       if not block_text:
         continue
 
       if type(block_infos[block_num]) is list:
         # The block is of the type output from _break_down_block().
-        _write_block(output_check_lines,
+        used_prefixes |= _write_block(output_check_lines,
                                       block_infos[block_num],
                                       not_prefix_set,
                                       common_prefix,
                                       prefix_pad)
         break
       elif block_infos[block_num][block_text]:
         # _break_down_block() was unable to do do anything so output the block
         # as-is.
         lines = block_text.split('\n')
         for prefix in block_infos[block_num][block_text]:
-          _write_block(output_check_lines,
+          used_prefixes |= _write_block(output_check_lines,
                                         [(prefix, line) for line in lines],
                                         not_prefix_set,
                                         common_prefix,
                                         prefix_pad)
 
+  unused_prefixes = (prefix_set - not_prefix_set) - used_prefixes
+  if unused_prefixes:
+    raise Error('unused prefixes: {}'.format(sorted(unused_prefixes)))
+
   if output_check_lines:
     output_lines.insert(0, ADVERT)
     output_lines.extend(output_check_lines)
 
   # The file should not end with two newlines. It creates unnecessary churn.
   while len(output_lines) > 0 and output_lines[-1] == '':
     output_lines.pop()