[RISCV][MachineVerifier] Use RegUnit for register liveness checking #115980

BeMg · 2024-11-13T02:48:53Z

For the RISC-V target, V14_V15 are not subregisters of v14m4, even though they share some registers. Currently, the MachineVerifier reports an error when checking register liveness for segment load/store operations.

This patch adds additional register liveness checking, using RegUnit instead of subregisters, to prevent this error.

…long to live super-register For the RISC-V target, V14_V15 are not belong to subregisters of v14m4, even though they share some registers. Currently, the MachineVerifier reports an error when checking register liveness for segment load/store operations. This patch adds additional register liveness checking, using RegUnit instead of subregisters, to prevent this error.

BeMg · 2024-11-13T02:49:23Z

Without this patch, the testcase will report

*** Bad machine code: Using an undefined physical register ***
- function:    func
- basic block: %bb.0  (0xa3bc568)
- instruction: $v20m2 = VMV2R_V $v14m2, implicit $v12_v13_v14_v15_v16
- operand 1:   $v14m2
LLVM ERROR: Found 1 machine code errors.

llvm/test/CodeGen/RISCV/subreg-liveness.mir

arsenm · 2024-11-13T17:15:48Z

llvm/lib/CodeGen/MachineVerifier.cpp

@@ -3035,6 +3035,16 @@ void MachineVerifier::checkLiveness(const MachineOperand *MO, unsigned MONum) {

            if (llvm::is_contained(TRI->subregs(MOP.getReg()), Reg))
              Bad = false;
+
+            if (any_of(TRI->subregs(MOP.getReg()),


The outer any_of should be redundant with checking the regunits of the raw register. This should also be redundant with the is_contained in subregs above.

Overall this whole verifier liveness tracking is not well implemented. BBInfo and the rest of the infrastructure should be implemented directly in terms of register units

We would encounter problems if we were to drop the outer any_of with subregs().

Take this testcase as example:

https://github.com/llvm/llvm-project/blob/main/llvm/test/MachineVerifier/AMDGPU/verifier-implicit-virtreg-invalid-physreg-liveness.mir

# RUN: not --crash llc -mtriple=amdgcn-amd-amdhsa -verify-machineinstrs -o /dev/null %s 2>&1 | FileCheck -check-prefix=ERROR %s # When the verifier was detecting the invalid liveness for vcc, it would assert when trying to iterate the subregisters of the implicit virtual register use. # ERROR: *** Bad machine code: Using an undefined physical register *** # ERROR: instruction: S_ENDPGM 0, implicit %0:vgpr_32, implicit $vcc # ERROR: operand 2: implicit $vcc ... name: invalid_implicit_physreg_use_with_implicit_virtreg tracksRegLiveness: true body: | bb.0: %0:vgpr_32 = IMPLICIT_DEF S_ENDPGM 0, implicit %0, implicit $vcc ...

The verification process will try to check if $vcc belongs to $vcc, which will always evaluate to true.

To avoid this problem, I modified to check only when MOP.getReg() does not equal Reg.

if (MOP.getReg() != Reg && all_of(TRI->regunits(Reg), [&](const MCRegUnit RegUnit) { return llvm::is_contained(TRI->regunits(MOP.getReg()), RegUnit); }))

llvm/test/MachineVerifier/RISCV/subreg-liveness.mir

arsenm · 2024-11-16T00:56:00Z

llvm/lib/CodeGen/MachineVerifier.cpp

@@ -3035,6 +3035,13 @@ void MachineVerifier::checkLiveness(const MachineOperand *MO, unsigned MONum) {

            if (llvm::is_contained(TRI->subregs(MOP.getReg()), Reg))
              Bad = false;
+


Above is_contained still redundant?

Yes, thanks for the reminder. Dropped.

llvmbot · 2024-11-22T19:01:51Z

@llvm/pr-subscribers-llvm-regalloc

Author: Piyou Chen (BeMg)

Changes

For the RISC-V target, V14_V15 are not subregisters of v14m4, even though they share some registers. Currently, the MachineVerifier reports an error when checking register liveness for segment load/store operations.

This patch adds additional register liveness checking, using RegUnit instead of subregisters, to prevent this error.

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

2 Files Affected:

(modified) llvm/lib/CodeGen/MachineVerifier.cpp (+5-1)
(added) llvm/test/MachineVerifier/RISCV/subreg-liveness.mir (+26)

diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index 3910046a1652b1..b08a93ae9a6d58 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -3033,7 +3033,11 @@ void MachineVerifier::checkLiveness(const MachineOperand *MO, unsigned MONum) {
             if (!MOP.getReg().isPhysical())
               continue;
 
-            if (llvm::is_contained(TRI->subregs(MOP.getReg()), Reg))
+            if (MOP.getReg() != Reg &&
+                all_of(TRI->regunits(Reg), [&](const MCRegUnit RegUnit) {
+                  return llvm::is_contained(TRI->regunits(MOP.getReg()),
+                                            RegUnit);
+                }))
               Bad = false;
           }
         }
diff --git a/llvm/test/MachineVerifier/RISCV/subreg-liveness.mir b/llvm/test/MachineVerifier/RISCV/subreg-liveness.mir
new file mode 100644
index 00000000000000..cb73f500ddc218
--- /dev/null
+++ b/llvm/test/MachineVerifier/RISCV/subreg-liveness.mir
@@ -0,0 +1,26 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
+# RUN: llc -mtriple=riscv64 -mattr=+v -run-pass=none %s -o - | FileCheck %s
+
+# During the MachineVerifier, it assumes that used registers have been defined
+# In this test case, while $v12_v13_v14_v15_v16 covers $v14_v15,
+# $v14_v15 is not a sub-register of $v14m2 even though they share the same register.
+# This corner case can be resolved by checking the register using RegUnit.
+
+...
+---
+name:            func
+tracksRegLiveness: true
+tracksDebugUserValues: true
+body:             |
+  bb.0:
+    liveins: $v0, $v8, $v9, $v10, $v11
+
+    ; CHECK-LABEL: name: func
+    ; CHECK: liveins: $v0, $v8, $v9, $v10, $v11
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: renamable $v16m2 = PseudoVMV_V_I_M2 undef renamable $v16m2, 0, -1, 3 /* e8 */, 0 /* tu, mu */, implicit $vl, implicit $vtype
+    ; CHECK-NEXT: $v20m2 = VMV2R_V $v14m2, implicit $v12_v13_v14_v15_v16
+    renamable $v16m2 = PseudoVMV_V_I_M2 undef renamable $v16m2, 0, -1, 3 /* e8 */, 0 /* tu, mu */, implicit $vl, implicit $vtype
+    $v20m2 = VMV2R_V $v14m2, implicit $v12_v13_v14_v15_v16
+
+...

llvm-ci · 2024-11-25T04:49:10Z

LLVM Buildbot has detected a new failure on builder llvm-nvptx-nvidia-ubuntu running on as-builder-7 while building llvm at step 6 "test-build-unified-tree-check-llvm".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/180/builds/8959