[update_mir_test_checks] Handle multiple defs of vreg

[jayfoad]

When (post-SSA) MIR has multiple defs of the same vreg,
update_mir_test_checks would use different variable names for each def
like this, where DEF and DEF1 both refer to %0:

    %0:gr32 = IMPLICIT_DEF
    %0:gr32 = IMPLICIT_DEF
-->
    ; CHECK: [[DEF:%[0-9]+]]:gr32 = IMPLICIT_DEF
    ; CHECK-NEXT: [[DEF1:%[0-9]+]]:gr32 = IMPLICIT_DEF

This should be harmless, but it messed up the way that mangle_vreg
counts the number of names in vreg_map to come up with a new numeric
suffix, such that you could get the same variable name for different
vregs, like this, where DEF2 refers to both %0 and %2:

    %0:gr32 = IMPLICIT_DEF
    %1:gr32 = IMPLICIT_DEF
    %0:gr32 = IMPLICIT_DEF
    %2:gr32 = IMPLICIT_DEF
-->
    ; CHECK: [[DEF:%[0-9]+]]:gr32 = IMPLICIT_DEF
    ; CHECK-NEXT: [[DEF1:%[0-9]+]]:gr32 = IMPLICIT_DEF
    ; CHECK-NEXT: [[DEF2:%[0-9]+]]:gr32 = IMPLICIT_DEF
    ; CHECK-NEXT: [[DEF2:%[0-9]+]]:gr32 = IMPLICIT_DEF

Fix this by always using the same variable name for the same vreg.

[jayfoad]

Please review the second commit. The first one is #66482.

[llvmbot]

@llvm/pr-subscribers-testing-tools

Changes

XFAIL it until it is fixed by an upcoming commit.

--
Full diff: https://github.com/llvm/llvm-project/pull/66483.diff

3 Files Affected:

• (added) llvm/test/tools/UpdateTestChecks/update_mir_test_checks/Inputs/multiple-defs.mir (+12)
• (added) llvm/test/tools/UpdateTestChecks/update_mir_test_checks/multiple-defs.test (+5)
• (modified) llvm/utils/update_mir_test_checks.py (+5-2)

diff --git a/llvm/test/tools/UpdateTestChecks/update_mir_test_checks/Inputs/multiple-defs.mir b/llvm/test/tools/UpdateTestChecks/update_mir_test_checks/Inputs/multiple-defs.mir
new file mode 100644
index 000000000000000..ff12ed7770beca0
--- /dev/null
+++ b/llvm/test/tools/UpdateTestChecks/update_mir_test_checks/Inputs/multiple-defs.mir
@@ -0,0 +1,12 @@
+# RUN: llc -mtriple=x86_64 -run-pass=none -verify-machineinstrs %s -o - | FileCheck %s
+
+---
+name: test
+body: |
+  bb.0:
+    %0:gr32 = IMPLICIT_DEF
+    %1:gr32 = IMPLICIT_DEF
+    %0:gr32 = IMPLICIT_DEF
+    %2:gr32 = IMPLICIT_DEF
+    KILL %0, %2
+...
diff --git a/llvm/test/tools/UpdateTestChecks/update_mir_test_checks/multiple-defs.test b/llvm/test/tools/UpdateTestChecks/update_mir_test_checks/multiple-defs.test
new file mode 100644
index 000000000000000..1009bcd7571159c
--- /dev/null
+++ b/llvm/test/tools/UpdateTestChecks/update_mir_test_checks/multiple-defs.test
@@ -0,0 +1,5 @@
+## Check that update_mir_test_checks handles multiple definitions of the same
+## virtual register (after coming out of SSA form).
+
+# RUN: cp -f %S/Inputs/multiple-defs.mir %t.mir && %update_mir_test_checks %t.mir
+# RUN: FileCheck %t.mir < %t.mir
diff --git a/llvm/utils/update_mir_test_checks.py b/llvm/utils/update_mir_test_checks.py
index 815738b23402310..8a539b5fb5ce4bc 100755
--- a/llvm/utils/update_mir_test_checks.py
+++ b/llvm/utils/update_mir_test_checks.py
@@ -204,8 +204,11 @@ def build_function_info_dictionary(
             m = VREG_DEF_RE.match(func_line)
             if m:
                 for vreg in VREG_RE.finditer(m.group("vregs")):
-                    name = mangle_vreg(m.group("opcode"), vreg_map.values())
-                    vreg_map[vreg.group(1)] = name
+                    if vreg.group(1) in vreg_map:
+                        name = vreg_map[vreg.group(1)]
+                    else:
+                        name = mangle_vreg(m.group("opcode"), vreg_map.values())
+                        vreg_map[vreg.group(1)] = name
                     func_line = func_line.replace(
                         vreg.group(1), "[[{}:%[0-9]+]]".format(name), 1
                     )

[arsenm]

Title here is wrong?

[jayfoad]

Title here is wrong?

Oh, I misunderstood gh pr create --fill-first. I guess I need --fill-last.

[nhaehnle]

Shouldn't the test demonstrate which CHECK lines are added? Other tests add a .expected file.

Apart from that, the change looks pretty reasonable to me.

[jayfoad]

Shouldn't the test demonstrate which CHECK lines are added? Other tests add a .expected file.

OK, since @nikic also mentioned that in #66482, I'll do it.

[jayfoad]

Shouldn't the test demonstrate which CHECK lines are added? Other tests add a .expected file.

OK, since @nikic also mentioned that in #66482, I'll do it.

Done.

[nikic]

LGTM