[PATCH] D48276: [llvm-mca] Stricter checking from update_mca_test_checks.py

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 25 11:50:32 PDT 2018


lebedev.ri accepted this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

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.



================
Comment at: utils/update_mca_test_checks.py:324-326
+      if block_text and not current_set:
+        raise Error(
+          'block not captured by existing prefixes:\n\n{}'.format(block_text))
----------------
I'm not sure, is this related to the rest of the diff?


================
Comment at: utils/update_mca_test_checks.py:434
         # The block is of the type output from _break_down_block().
-        _write_block(output_check_lines,
-                     block_infos[block_num],
-                     not_prefix_set,
-                     common_prefix,
-                     prefix_pad)
+        used_prefixes.update(_write_block(output_check_lines,
+                                          block_infos[block_num],
----------------
Why `update`? The return isn't used.
Could you just do `used_prefixes |= `?
Would be more readable, too.


https://reviews.llvm.org/D48276





More information about the llvm-commits mailing list