[utils] fixing update_mir_test_checks.py's greediness for `registers:` field

[rtereshin]

Summary

utils/update_mir_test_checks.py may cross machine function boundaries while parsing a *.mir test looking for registers: YAML-field a little too greedily than it actually needs. If there is a MF w/o registers: field followed by a MF with one it will parse both of them with the name of the first one, and registers and body of the second one and eventually crash.

This patch also adds a LIT-test for utils/update_mir_test_checks.py itself to ensure we don't regress on this or, frankly, anything else.

[rtereshin]

@bogner

I've also double checked that the test actually fails. On the original tool it fails with

-- Testing: 1 tests, 1 threads --
FAIL: LLVM :: CodeGen/AArch64/actual_test.mir (1 of 1)
******************** TEST 'LLVM :: CodeGen/AArch64/actual_test.mir' FAILED ********************
Script:
--
sed -e 's/^# RUN-NESTED:/# RUN:/' /Volumes/Data/llvm/test/CodeGen/AArch64/actual_test.mir | sed -e 's/^# SOURCE.*$//' > /Volumes/Data/llvm/build/obj/test/CodeGen/AArch64/Output/actual_test.mir.tmp    && /Volumes/Data/llvm/test/CodeGen/AArch64/../../../utils/update_mir_test_checks.py --add-vreg-checks      /Volumes/Data/llvm/build/obj/test/CodeGen/AArch64/Output/actual_test.mir.tmp 2>&1 | /Volumes/Data/llvm/build/obj/bin/FileCheck /Volumes/Data/llvm/test/CodeGen/AArch64/actual_test.mir --check-prefix=STDERR    && cat /Volumes/Data/llvm/build/obj/test/CodeGen/AArch64/Output/actual_test.mir.tmp | /Volumes/Data/llvm/build/obj/bin/FileCheck /Volumes/Data/llvm/test/CodeGen/AArch64/actual_test.mir --check-prefix=SOURCE
--
Exit Code: 1

Command Output (stderr):
--
<stdin>:2:1: error: STDERR-NOT: string occurred!
WARNING: /Volumes/Data/llvm/build/obj/test/CodeGen/AArch64/Output/actual_test.mir.tmp: Error processing file
^
/Volumes/Data/llvm/test/CodeGen/AArch64/actual_test.mir:14:15: note: STDERR-NOT: pattern specified here
# STDERR-NOT: WARNING
              ^

--

********************
Testing Time: 0.21s
********************
Failing Tests (1):
    LLVM :: CodeGen/AArch64/actual_test.mir

  Unexpected Failures: 1

If the issue with registers: field "fixed" by making its regexp non just optional, but non-greedy-optional (??) it will still fail:

-- Testing: 1 tests, 1 threads --
FAIL: LLVM :: CodeGen/AArch64/actual_test.mir (1 of 1)
******************** TEST 'LLVM :: CodeGen/AArch64/actual_test.mir' FAILED ********************
Script:
--
sed -e 's/^# RUN-NESTED:/# RUN:/' /Volumes/Data/llvm/test/CodeGen/AArch64/actual_test.mir | sed -e 's/^# SOURCE.*$//' > /Volumes/Data/llvm/build/obj/test/CodeGen/AArch64/Output/actual_test.mir.tmp    && /Volumes/Data/llvm/test/CodeGen/AArch64/../../../utils/update_mir_test_checks.py --add-vreg-checks      /Volumes/Data/llvm/build/obj/test/CodeGen/AArch64/Output/actual_test.mir.tmp 2>&1 | /Volumes/Data/llvm/build/obj/bin/FileCheck /Volumes/Data/llvm/test/CodeGen/AArch64/actual_test.mir --check-prefix=STDERR    && cat /Volumes/Data/llvm/build/obj/test/CodeGen/AArch64/Output/actual_test.mir.tmp | /Volumes/Data/llvm/build/obj/bin/FileCheck /Volumes/Data/llvm/test/CodeGen/AArch64/actual_test.mir --check-prefix=SOURCE
--
Exit Code: 1

Command Output (stderr):
--
/Volumes/Data/llvm/test/CodeGen/AArch64/actual_test.mir:65:16: error: expected string not found in input
# SOURCE-NEXT: ; CHECK: registers:
               ^
<stdin>:126:2: note: scanning from here
 ; CHECK: liveins: $w0, $w1
 ^

--

********************
Testing Time: 0.21s
********************
Failing Tests (1):
    LLVM :: CodeGen/AArch64/actual_test.mir

  Unexpected Failures: 1

[bogner]

I like the idea of adding tests for update_mir_test_checks itself, but I think we should stick to focused tests rather than just adding one that tries to test everything. I also think this isn't a great place for it, maybe we could add a test/tools/update_checks folder for tests for all of the update_*_tests scripts? We might need to fake out llc to get away with something like that though...

test/CodeGen/AArch64/test-update_mir_test_checks.py-itself.mir

11–14

Is it necessary to check the stderr in this test?

[rtereshin]

As --add-vreg-checks option support was removed by http://llvm.org/viewvc/llvm-project?view=revision&revision=326284 rendering this largely outdated, closing it.

We should probably revisit testing, though, but for that it's probably enough to have https://reviews.llvm.org/D43604 hanging around anyway.

test/CodeGen/AArch64/test-update_mir_test_checks.py-itself.mir

+# UNSUPPORTED: system-windows
+#
+# RUN: sed -e 's/^# RUN-NESTED:/# RUN:/' %s | sed -e 's/^# SOURCE.*$//' > %t \
+# RUN:   && %S/../../../utils/update_mir_test_checks.py --add-vreg-checks \
+# RUN:     %t 2>&1 | FileCheck %s --check-prefix=STDERR \
+# RUN:   && cat %t | FileCheck %s --check-prefix=SOURCE
+#
+# RUN-NESTED: llc -mtriple aarch64-apple-ios -run-pass instruction-select %s \
+# RUN-NESTED:     -x mir -verify-machineinstrs -simplify-mir -o - | FileCheck %s
+
+# STDERR-NOT:  WARNING
+# STDERR:      WARNING: [[TMP_FILE_NAME:/.*\.tmp]]: Skipping non-llc RUN line:
+# STDERR-SAME:     sed -e {{.*}} RUN-NESTED{{.*}}SOURCE{{.*}}STDERR{{.*}}SOURCE
+# STDERR-NOT:  WARNING
+
+# SOURCE:      WARNING
+# SOURCE-NEXT: WARNING
+# SOURCE-NEXT:     sed
+# SOURCE-NEXT: WARNING
+#
+# SOURCE-LABEL: --- |
+# SOURCE-NEXT:   target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
+# SOURCE-NEXT:   target triple = "aarch64-apple-ios"
+# SOURCE-NOT: SOURCE
+# SOURCE:        define void @return_only() #0 {
+# SOURCE-NEXT:   entry:
+# SOURCE-NEXT:     ret void
+# SOURCE-NEXT:   }
+# SOURCE-NOT: SOURCE
+# SOURCE:        define i32 @some_math(i32 %a, i32 %b) #0 {
+# SOURCE-NEXT:   entry:
+# SOURCE-NEXT:     %add = add i32 %b, %a
+# SOURCE-NEXT:     ret i32 %add
+# SOURCE-NEXT:   }
+# SOURCE-NOT: SOURCE
+# SOURCE:        attributes #0 = { nounwind readnone ssp }
+# SOURCE-NEXT: ...
+# SOURCE-NEXT: ---
+# SOURCE-NEXT: name:            return_only
+# SOURCE-NEXT: alignment:       2
+# SOURCE-NEXT: legalized:       true
+# SOURCE-NEXT: regBankSelected: true
+# SOURCE-NEXT: tracksRegLiveness: true
+# SOURCE-NEXT: body:             |
+# SOURCE-NEXT:   bb.1.entry:
+# SOURCE-NEXT:     ; CHECK-LABEL: name: return_only
+# SOURCE-NEXT:     ; CHECK: RET_ReallyLR
+# SOURCE-NEXT:     RET_ReallyLR
+# SOURCE-NEXT: ...
+# SOURCE-NEXT: ---
+# SOURCE-NEXT: name:            some_math
+# SOURCE-NEXT: alignment:       2
+# SOURCE-NEXT: legalized:       true
+# SOURCE-NEXT: regBankSelected: true
+# SOURCE-NEXT: tracksRegLiveness: true
+# SOURCE-NEXT: registers:
+# SOURCE-NEXT:   - { id: 0, class: gpr }
+# SOURCE-NEXT:   - { id: 1, class: gpr }
+# SOURCE-NEXT:   - { id: 2, class: gpr }
+# SOURCE-NEXT: body:             |
+# SOURCE-NEXT:   bb.1.entry:
+# SOURCE-NEXT:     liveins: $w0, $w1
+# SOURCE-NOT: SOURCE
+# SOURCE:          ; CHECK-LABEL: name: some_math
+# SOURCE-NEXT:     ; CHECK: registers:
+# SOURCE-NEXT:     ; CHECK-NEXT: id: 0, class: gpr32
+# SOURCE-NEXT:     ; CHECK-NEXT: id: 1, class: gpr32
+# SOURCE-NEXT:     ; CHECK-NEXT: id: 2, class: gpr32
+# SOURCE-NEXT:     ; CHECK: liveins: $w0, $w1
+# SOURCE-NEXT:     ; CHECK: {{\[}}[COPY:%[0-9]+]]:gpr32 = COPY $w0
+# SOURCE-NEXT:     ; CHECK: {{\[}}[COPY1:%[0-9]+]]:gpr32 = COPY $w1
+# SOURCE-NEXT:     ; CHECK: {{\[}}[ADDWrr:%[0-9]+]]:gpr32 = ADDWrr
+# SOURCE-SAME:     {{\[}}[COPY1]], {{\[}}[COPY]]
+# SOURCE-NEXT:     ; CHECK: $w0 = COPY {{\[}}[ADDWrr]]
+# SOURCE-NEXT:     ; CHECK: RET_ReallyLR implicit $w0
+# SOURCE-NEXT:     %0:gpr(s32) = COPY $w0
+# SOURCE-NEXT:     %1:gpr(s32) = COPY $w1
+# SOURCE-NEXT:     %2:gpr(s32) = G_ADD %1, %0
+# SOURCE-NEXT:     $w0 = COPY %2(s32)
+# SOURCE-NEXT:     RET_ReallyLR implicit $w0
+# SOURCE-NEXT: ...
+--- |
+  target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
+  target triple = "aarch64-apple-ios"
+
+  define void @return_only() #0 {
+  entry:
+    ret void
+  }
+
+  define i32 @some_math(i32 %a, i32 %b) #0 {
+  entry:
+    %add = add i32 %b, %a
+    ret i32 %add
+  }
+
+  attributes #0 = { nounwind readnone ssp }
+...
+---
+name:            return_only
+alignment:       2
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+body:             |
+  bb.1.entry:
+    RET_ReallyLR
+...
+---
+name:            some_math
+alignment:       2
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: gpr }
+  - { id: 1, class: gpr }
+  - { id: 2, class: gpr }
+body:             |
+  bb.1.entry:
+    liveins: $w0, $w1
+
+    %0:gpr(s32) = COPY $w0
+    %1:gpr(s32) = COPY $w1
+    %2:gpr(s32) = G_ADD %1, %0
+    $w0 = COPY %2(s32)
+    RET_ReallyLR implicit $w0
+...

utils/update_mir_test_checks.py

 IR_FUNC_NAME_RE = re.compile(
     r'^\s*define\s+(?:internal\s+)?[^@]*@(?P<func>[A-Za-z0-9_.]+)\s*\(')
 IR_PREFIX_DATA_RE = re.compile(r'^ *(;|$)')
 
 MIR_FUNC_RE = re.compile(
     r'^---$'
     r'\n'
     r'^ *name: *(?P<func>[A-Za-z0-9_.-]+)$'
-    r'(?:.*?(?P<vregs>^ *registers: *(?:\n *- {[^\n]+$)*))?'
+    r'(?:(?:.(?!\.\.\.))*?(?P<vregs>^ *registers: *(?:\n *- {[^\n]+$)*))?'
     r'.*?'
     r'^ *body: *\|\n'
     r'(?P<body>.*?)\n'
     r'^\.\.\.$',
     flags=(re.M | re.S))
 
 class LLC:
     def __init__(self, bin):