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

Michael Maitland via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 3 07:50:59 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() &&
----------------
michaelmaitland wrote:

We do need some ability to copy physical registers into virtual registers and vice versa.

The way we accomplish this, I am not sure, and I am interested in having a discussion on it. I have updated the code here to allow copy from physical -> virtual when we know the min size is at least as big. However, I am not confident that this is the correct approach. The first reason I am not sure this is the correct approach is because this dual for return values will not work:

```
if (SrcReg.isVirtual() && DstReg.isPhysical() &&
     SrcSize.isScalable() && !DstSize.isScalable() &&
     TypeSize::isKnownLE(DstSize, SrcSize)
```

The problem here is that `TypeSize::isKnownLE(DstSize, SrcSize)` will always be false.

The second reason I am not sure this is the correct approach is because it isn't clear to me what happens to elements in the scalable vector that are past the size of the physical register. For example, v8 in the test case below reports a size of 64 bits, but %0 reports a size of vscale x 8. If vscale is bigger than 8, then I'm not sure what goes into the rest of the elements.

Does anyone have any opinion on a better approach?

@davemgreen, does marking the physical registers as scalable solve this problem? IIUC marking v8 here as scalable would mean that it has size `vscale x 64`? I'm not sure that this would match with the `vscale x 8` size that we're using for the virtual register though.

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


More information about the llvm-commits mailing list