From 009398b3b37f7d653b4371120944f74cad934992 Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper@sifive.com>
Date: Wed, 18 Sep 2024 18:31:16 -0700
Subject: [PATCH] [MachineVerifier] Improve checks for G_INSERT_SUBVECTOR.
 (#109209)

-Improve messages.
-Remove redundant checks that are handled in generic code.
-Add check that the subvector is smaller than the vector.
-Add checks that subvector is smaller than the vector.
---
 llvm/lib/CodeGen/MachineVerifier.cpp          | 32 ++++++++++++-------
 .../test_g_insert_subvector.mir               | 28 +++++++++++++---
 2 files changed, 45 insertions(+), 15 deletions(-)

diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index 651de06cfac25d..27664207d1e696 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1710,7 +1710,6 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
     }
 
     LLT DstTy = MRI->getType(MI->getOperand(0).getReg());
-    LLT Src0Ty = MRI->getType(Src0Op.getReg());
     LLT Src1Ty = MRI->getType(Src1Op.getReg());
 
     if (!DstTy.isVector()) {
@@ -1718,33 +1717,44 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
       break;
     }
 
-    if (!Src0Ty.isVector()) {
-      report("First source must be a vector", MI);
+    if (!Src1Ty.isVector()) {
+      report("Second source must be a vector", MI);
       break;
     }
 
-    if (!Src1Ty.isVector()) {
-      report("Second source must be a vector", MI);
+    if (DstTy.getElementType() != Src1Ty.getElementType()) {
+      report("Element type of vectors must be the same", MI);
       break;
     }
 
-    if (DstTy != Src0Ty) {
-      report("Destination type must match the first source vector type", MI);
+    if (Src1Ty.isScalable() != DstTy.isScalable()) {
+      report("Vector types must both be fixed or both be scalable", MI);
       break;
     }
 
-    if (Src0Ty.getElementType() != Src1Ty.getElementType()) {
-      report("Element type of source vectors must be the same", MI);
+    if (ElementCount::isKnownGT(Src1Ty.getElementCount(),
+                                DstTy.getElementCount())) {
+      report("Second source must be smaller than destination vector", MI);
       break;
     }
 
-    if (IndexOp.getImm() != 0 &&
-        IndexOp.getImm() % Src1Ty.getElementCount().getKnownMinValue() != 0) {
+    uint64_t Idx = IndexOp.getImm();
+    uint64_t Src1MinLen = Src1Ty.getElementCount().getKnownMinValue();
+    if (IndexOp.getImm() % Src1MinLen != 0) {
       report("Index must be a multiple of the second source vector's "
              "minimum vector length",
              MI);
       break;
     }
+
+    uint64_t DstMinLen = DstTy.getElementCount().getKnownMinValue();
+    if (Idx >= DstMinLen || Idx + Src1MinLen > DstMinLen) {
+      report("Subvector type and index must not cause insert to overrun the "
+             "vector being inserted into",
+             MI);
+      break;
+    }
+
     break;
   }
   case TargetOpcode::G_EXTRACT_SUBVECTOR: {
diff --git a/llvm/test/MachineVerifier/test_g_insert_subvector.mir b/llvm/test/MachineVerifier/test_g_insert_subvector.mir
index 62ddd28919b205..84cb249d74eb79 100644
--- a/llvm/test/MachineVerifier/test_g_insert_subvector.mir
+++ b/llvm/test/MachineVerifier/test_g_insert_subvector.mir
@@ -11,9 +11,11 @@ body:             |
     %1:_(<vscale x 2 x s32>) = G_IMPLICIT_DEF
     %2:_(<vscale x 1 x s32>) = G_IMPLICIT_DEF
 
+    ; CHECK: generic instruction must use register operands
     ; CHECK: G_INSERT_SUBVECTOR first source must be a register
     %3:_(<vscale x 2 x s32>) = G_INSERT_SUBVECTOR 1, %2, 0
 
+    ; CHECK: generic instruction must use register operands
     ; CHECK: G_INSERT_SUBVECTOR second source must be a register
     %4:_(<vscale x 2 x s32>) = G_INSERT_SUBVECTOR %1, 1, 0
 
@@ -23,18 +25,18 @@ body:             |
     ; CHECK: Destination type must be a vector
     %6:_(s32) = G_INSERT_SUBVECTOR %1, %2, 0
 
-    ; CHECK: First source must be a vector
+    ; CHECK: Type mismatch in generic instruction
     %7:_(<vscale x 2 x s32>) = G_INSERT_SUBVECTOR %0, %2, 0
 
     ; CHECK: Second source must be a vector
     %8:_(<vscale x 2 x s32>) = G_INSERT_SUBVECTOR %1, %0, 0
 
-    ; CHECK: Destination type must match the first source vector type
+    ; CHECK: Type mismatch in generic instruction
     %9:_(<vscale x 2 x s32>) = G_INSERT_SUBVECTOR %2, %1, 0
 
     %10:_(<vscale x 1 x s16>) = G_IMPLICIT_DEF
 
-    ; CHECK: Element type of source vectors must be the same
+    ; CHECK: Element type of vectors must be the same
     %11:_(<vscale x 2 x s32>) = G_INSERT_SUBVECTOR %1, %10, 0
 
     %12:_(<vscale x 4 x s32>) = G_IMPLICIT_DEF
@@ -43,5 +45,23 @@ body:             |
     %13:_(<vscale x 4 x s32>) = G_INSERT_SUBVECTOR %12, %1, 3
 
     ; CHECK: Index must be a multiple of the second source vector's minimum vector length
-    %13:_(<vscale x 4 x s32>) = G_INSERT_SUBVECTOR %12, %1, 1
+    %14:_(<vscale x 4 x s32>) = G_INSERT_SUBVECTOR %12, %1, 1
+
+    %15:_(<vscale x 4 x s32>) = G_IMPLICIT_DEF
+
+    ; CHECK: Second source must be smaller than destination vector
+    %16:_(<vscale x 2 x s32>) = G_INSERT_SUBVECTOR %1, %15, 0
+
+    ; CHECK: Subvector type and index must not cause insert to overrun the vector being inserted into
+    %17:_(<vscale x 4 x s32>) = G_INSERT_SUBVECTOR %12, %1, 4
+
+    %18:_(<vscale x 3 x s32>) = G_IMPLICIT_DEF
+
+    ; CHECK: Subvector type and index must not cause insert to overrun the vector being inserted into
+    %19:_(<vscale x 3 x s32>) = G_INSERT_SUBVECTOR %18, %1, 2
+
+    %20:_(<2 x s32>) = G_IMPLICIT_DEF
+
+    ; CHECK: Vector types must both be fixed or both be scalable
+    %21:_(<vscale x 1 x s32>) = G_INSERT_SUBVECTOR %12, %20, 2
 ...