[PATCH] D135579: utils/update_mir_test_checks.py: allow checking fixedStack in .mir files

Gaƫtan Bossu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 3 05:51:36 PDT 2022


gbossu added inline comments.


================
Comment at: llvm/test/tools/UpdateTestChecks/update_mir_test_checks/Inputs/print-stack-first.mir:3-5
+# Note that this file isn't a test in itself (Inputs/ is excluded from lit's
+# test discovery). Instead, it is an input to the print-stack.test test that
+# verifies the --print-fixed-stack option of update_mir_test_checks.py.
----------------
nhaehnle wrote:
> This comment isn't really necessary.
I do agree, but this was brought to my attention that such a comment would be useful to people not too familiar with llvm's test structure. If you feel it's not needed or too verbose, I do not mind removing it.


================
Comment at: llvm/utils/update_mir_test_checks.py:247
 def add_check_lines(test, output_lines, prefix, func_name, single_bb,
-                    func_body):
+                    func_info :FunctionInfo, args):
+    func_body = func_info.body.splitlines()
----------------
nhaehnle wrote:
> I don't think we really have a Python coding style guide, but this should probably be `func_info: FunctionInfo`
Will change


================
Comment at: llvm/utils/update_mir_test_checks.py:269
+            filecheck_directive = check + '-NEXT'
+            first_check = False
+            output_lines.append('{}: {}'.format(filecheck_directive, stack_line))
----------------
nhaehnle wrote:
> This assignment seems pointless?
Good catch, I think it's a remnant from some conflict resolution...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135579/new/

https://reviews.llvm.org/D135579



More information about the llvm-commits mailing list