[PATCH] D58261: GlobalISel: Fix inadequate verification of g_build_vector

[Matt Arsenault via Phabricator via llvm-commits]

arsenm created this revision.
arsenm added reviewers: aemerson, paquette, aditya_nandakumar, dsanders, volkan.
Herald added subscribers: Petar.Avramovic, kristof.beyls, rovka, wdng.

Testing based on the total size of the elements failed to catch a few
invalid scenarios, so explicitly check the number of elements/operands
and types.

     

This failed to catch situations like
<4 x s16> = G_BUILD_VECTOR s32, s32 since the total size added
up. This also would fail to catch an implicit conversion between
pointers and scalars.


https://reviews.llvm.org/D58261

Files:
  lib/CodeGen/MachineVerifier.cpp
  test/MachineVerifier/test_g_build_vector.mir


Index: test/MachineVerifier/test_g_build_vector.mir
===================================================================
--- test/MachineVerifier/test_g_build_vector.mir
+++ test/MachineVerifier/test_g_build_vector.mir
@@ -1,27 +1,41 @@
-#RUN: not llc -o - -global-isel -run-pass=none -verify-machineinstrs %s 2>&1 | FileCheck %s
+# RUN: not llc -o - -global-isel -run-pass=none -verify-machineinstrs %s 2>&1 | FileCheck %s
 # REQUIRES: aarch64-registered-target
---- |
-  target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
-  target triple = "aarch64-unknown-unknown"
-
-  define i32 @g_build_vector() {
-    ret i32 0
-  }
-
-...
 ---
 name:            g_build_vector
-legalized:       true
-regBankSelected: false
-selected:        false
 tracksRegLiveness: true
-registers:
-  - { id: 0, class: _, preferred-register: '' }
 liveins:
 body:             |
   bb.0:
-    ; CHECK: Bad machine code: G_BUILD_VECTOR src operands total size don't match dest size
+    %0:_(s32) = IMPLICIT_DEF
+
+    ; CHECK: Bad machine code: G_BUILD_VECTOR must produce a vector from scalar operands
+    %1:_(s32) = G_BUILD_VECTOR %0
+
+    %2:_(<2 x s32>) = IMPLICIT_DEF
+
+    ; CHECK: Bad machine code: G_BUILD_VECTOR must produce a vector from scalar operands
+    %3:_(<2 x s32>) = G_BUILD_VECTOR %2
+
+    ; CHECK: Bad machine code: G_BUILD_VECTOR must have an operand for each elemement
+    %4:_(<2 x s32>) = G_BUILD_VECTOR %0, %0, %0, %0
+
+    ; CHECK: Bad machine code: G_BUILD_VECTOR result element type must match source type
+    ; CHECK: Bad machine code: G_BUILD_VECTOR must have an operand for each elemement
+    %5:_(<4 x s16>) = G_BUILD_VECTOR %0, %0
+
+    %6:_(s16) = IMPLICIT_DEF
+
+    ; CHECK: Bad machine code: G_BUILD_VECTOR result element type must match source type
+    ; CHECK: Bad machine code: G_BUILD_VECTOR must have an operand for each elemement
+    %7:_(<2 x s32>) = G_BUILD_VECTOR %6, %6, %6, %6
+
+    %8:_(p0) = IMPLICIT_DEF
+
+    ; CHECK: Bad machine code: G_BUILD_VECTOR result element type must match source type
+    %9:_(<2 x s64>) = G_BUILD_VECTOR %8, %8
+
+    %10:_(s64) = IMPLICIT_DEF
 
-    %0(s32) = IMPLICIT_DEF
-    %1:_(<2 x s32>) = G_BUILD_VECTOR %0, %0, %0, %0
+    ; CHECK: Bad machine code: G_BUILD_VECTOR result element type must match source type
+    %11:_(<2 x p0>) = G_BUILD_VECTOR %10, %10
 ...
Index: lib/CodeGen/MachineVerifier.cpp
===================================================================
--- lib/CodeGen/MachineVerifier.cpp
+++ lib/CodeGen/MachineVerifier.cpp
@@ -1198,18 +1198,23 @@
     // must match the dest vector size.
     LLT DstTy = MRI->getType(MI->getOperand(0).getReg());
     LLT SrcEltTy = MRI->getType(MI->getOperand(1).getReg());
-    if (!DstTy.isVector() || SrcEltTy.isVector())
+    if (!DstTy.isVector() || SrcEltTy.isVector()) {
       report("G_BUILD_VECTOR must produce a vector from scalar operands", MI);
+      break;
+    }
+
+    if (DstTy.getElementType() != SrcEltTy)
+      report("G_BUILD_VECTOR result element type must match source type", MI);
+
+    if (DstTy.getNumElements() != MI->getNumOperands() - 1)
+      report("G_BUILD_VECTOR must have an operand for each elemement", MI);
+
     for (unsigned i = 2; i < MI->getNumOperands(); ++i) {
       if (MRI->getType(MI->getOperand(1).getReg()) !=
           MRI->getType(MI->getOperand(i).getReg()))
         report("G_BUILD_VECTOR source operand types are not homogeneous", MI);
     }
-    if (DstTy.getSizeInBits() !=
-        SrcEltTy.getSizeInBits() * (MI->getNumOperands() - 1))
-      report("G_BUILD_VECTOR src operands total size don't match dest "
-             "size.",
-             MI);
+
     break;
   }
   case TargetOpcode::G_BUILD_VECTOR_TRUNC: {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D58261.186931.patch
Type: text/x-patch
Size: 3737 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190214/da38af55/attachment.bin>