[llvm] 4832c88 - [MachineVerifier] Fix COPY check in MachineVerifier for scalable vectors

[Michael Maitland via llvm-commits]

Author: Michael Maitland
Date: 2023-11-07T14:49:38-08:00
New Revision: 4832c88e4956b3c7dc0a5064d946477448d5239d

URL: https://github.com/llvm/llvm-project/commit/4832c88e4956b3c7dc0a5064d946477448d5239d
DIFF: https://github.com/llvm/llvm-project/commit/4832c88e4956b3c7dc0a5064d946477448d5239d.diff

LOG: [MachineVerifier] Fix COPY check in MachineVerifier for scalable vectors

This change fixes #71518, which compared the KnownMinValue of the
scalable virtual register with the FixedSize of the physical register in
the wrong direction. It turns out that we cannot include this check at all since
it will lead to a false failures. Test cases are added to show that
the false failures no longer occur after this fix.

Added: 
    

Modified: 
    llvm/lib/CodeGen/MachineVerifier.cpp
    llvm/test/MachineVerifier/copy-scalable.mir

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index dc15f0d3b842304..da1d9c6f0679c7f 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1953,12 +1953,18 @@ void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) {
         DstSize = TRI->getRegSizeInBits(*DstRC);
     }
 
-    // If this is a copy from physical register to virtual register, and if the
-    // Dst is scalable and the Src is fixed, then the Dst can only hold the Src
-    // if the minimum size Dst can hold is at least as big as Src.
+    // The next two checks allow COPY between physical and virtual registers,
+    // when the virtual register has a scalable size and the physical register
+    // has a fixed size. These checks allow COPY between *potentialy* mismatched
+    // sizes. However, once RegisterBankSelection occurs, MachineVerifier should
+    // be able to resolve a fixed size for the scalable vector, and at that
+    // point this function will know for sure whether the sizes are mismatched
+    // and correctly report a size mismatch.
     if (SrcReg.isPhysical() && DstReg.isVirtual() && DstSize.isScalable() &&
-        !SrcSize.isScalable() &&
-        DstSize.getKnownMinValue() <= SrcSize.getFixedValue())
+        !SrcSize.isScalable())
+      break;
+    if (SrcReg.isVirtual() && DstReg.isPhysical() && SrcSize.isScalable() &&
+        !DstSize.isScalable())
       break;
 
     if (SrcSize.isNonZero() && DstSize.isNonZero() && SrcSize != DstSize) {

diff  --git a/llvm/test/MachineVerifier/copy-scalable.mir b/llvm/test/MachineVerifier/copy-scalable.mir
index f4088f7aed34dde..28d3e712455012d 100644
--- a/llvm/test/MachineVerifier/copy-scalable.mir
+++ b/llvm/test/MachineVerifier/copy-scalable.mir
@@ -3,7 +3,7 @@
 # REQUIRES: riscv64-registered-target
 
 ---
-name:            test_copy_fixed_to_scalable
+name:            test_copy_physical_to_virtual_nxv1s8
 legalized:       true
 regBankSelected: false
 selected:        false
@@ -15,9 +15,48 @@ body:             |
   bb.0:
     liveins: $v8
 
-    ; CHECK-LABEL: name: test_copy_fixed_to_scalable
+    ; CHECK-LABEL: name: test_copy_physical_to_virtual_nxv1s8
     ; CHECK: liveins: $v8
     ; CHECK-NEXT: {{  $}}
     ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(<vscale x 1 x s8>) = COPY $v8
     %0:_(<vscale x 1 x s8>) = COPY $v8
 ...
+
+---
+name:            test_copy_physical_to_virtual_nxv16s8
+legalized:         true
+tracksRegLiveness: true
+body:             |
+  bb.1.entry:
+    liveins: $v8
+    ; CHECK-LABEL: name: test_copy_physical_to_virtual_nxv16s8
+    ; CHECK: liveins: $v8
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(<vscale x 16 x s8>) = COPY $v8
+    %0:_(<vscale x 16 x s8>) = COPY $v8
+
+...
+
+---
+name:            test_copy_virtual_to_physical
+legalized:       true
+regBankSelected: false
+selected:        false
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: _, preferred-register: '' }
+liveins:
+body:             |
+  bb.0:
+    liveins: $v8
+
+    ; CHECK-LABEL: name: test_copy_virtual_to_physical
+    ; CHECK: liveins: $v8
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[DEF:%[0-9]+]]:_(<vscale x 1 x s8>) = IMPLICIT_DEF
+    ; CHECK-NEXT: $v8 = COPY [[DEF]](<vscale x 1 x s8>)
+    ; CHECK-NEXT: PseudoRET implicit $v8
+    %0:_(<vscale x 1 x s8>) = IMPLICIT_DEF
+    $v8 = COPY %0(<vscale x 1 x s8>)
+    PseudoRET implicit $v8
+...