[llvm] [CodeGen][MachineVerifier] Use TypeSize instead of unsigned for getRe… (PR #70881)

David Green via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 3 05:17:14 PDT 2023


================
@@ -1937,29 +1937,34 @@ void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) {
 
     // If we have only one valid type, this is likely a copy between a virtual
     // and physical register.
-    unsigned SrcSize = 0;
-    unsigned DstSize = 0;
+    TypeSize SrcSize = TRI->getRegSizeInBits(SrcReg, *MRI);
+    TypeSize DstSize = TRI->getRegSizeInBits(DstReg, *MRI);
     if (SrcReg.isPhysical() && DstTy.isValid()) {
       const TargetRegisterClass *SrcRC =
           TRI->getMinimalPhysRegClassLLT(SrcReg, DstTy);
       if (SrcRC)
         SrcSize = TRI->getRegSizeInBits(*SrcRC);
     }
 
-    if (SrcSize == 0)
-      SrcSize = TRI->getRegSizeInBits(SrcReg, *MRI);
-
     if (DstReg.isPhysical() && SrcTy.isValid()) {
       const TargetRegisterClass *DstRC =
           TRI->getMinimalPhysRegClassLLT(DstReg, SrcTy);
       if (DstRC)
         DstSize = TRI->getRegSizeInBits(*DstRC);
     }
 
-    if (DstSize == 0)
-      DstSize = TRI->getRegSizeInBits(DstReg, *MRI);
+    // 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.
+    if (DstSize.isScalable() && !SrcSize.isScalable() &&
+        DstSize.getKnownMinValue() <= SrcSize.getFixedValue())
+      break;
+    // If the Src is scalable and the Dst is fixed, then Dest can only hold
+    // the Src is known to fit in Dest
+    if (SrcSize.isScalable() && !DstSize.isScalable() &&
----------------
davemgreen wrote:

I agree that copys should be of the same size. As far as I understand the scalable vector registers have never been marked as "Scalable" though, only being given a size equal to the KnownMinValue.

The patch at https://github.com/davemgreen/llvm-project/commit/07e9bdd7a8a2205b8d13e3e5014fc0da84c4d7ac was enough to make the test case there work for a simple add. It went the route of marking the vector registers as scalable. I don't think that is necessary to make things work though, in that I can remove the Scalable defs from RegisterClasses and so long as it gets past the verifier it works for that small example still. SDAG never needed the registers to be scalable.

The sizes of some of the SVE scalable register types is a bit odd in places though, I was expecting them to all be vscale*128bit, and Im not sure how RISCV scalable vectors are defined.

https://github.com/llvm/llvm-project/pull/70881


More information about the llvm-commits mailing list