GlobalISel: Verify g_select

[arsenm]

Summary

Factor the common vector element consistency check many instructions
need out, although this makes the error messages worse.

[paquette]

LGTM

[arsenm]

r353112

lib/CodeGen/MachineVerifier.cpp

struct MachineVerifier {
     LiveIntervals *LiveInts;
     LiveStacks *LiveStks;
     SlotIndexes *Indexes;
 
     void visitMachineFunctionBefore();
     void visitMachineBasicBlockBefore(const MachineBasicBlock *MBB);
     void visitMachineBundleBefore(const MachineInstr *MI);
 
+    bool verifyVectorElementMatch(LLT Ty0, LLT Ty1, const MachineInstr *MI);
     void verifyPreISelGenericInstruction(const MachineInstr *MI);
     void visitMachineInstrBefore(const MachineInstr *MI);
     void visitMachineOperand(const MachineOperand *MO, unsigned MONum);
     void visitMachineInstrAfter(const MachineInstr *MI);
     void visitMachineBundleAfter(const MachineInstr *MI);
     void visitMachineBasicBlockAfter(const MachineBasicBlock *MBB);
     void visitMachineFunctionAfter();
 
void MachineVerifier::verifyInlineAsm(const MachineInstr *MI) {
   // All trailing operands must be implicit registers.
   for (unsigned e = MI->getNumOperands(); OpNo < e; ++OpNo) {
     const MachineOperand &MO = MI->getOperand(OpNo);
     if (!MO.isReg() || !MO.isImplicit())
       report("Expected implicit register after groups", &MO, OpNo);
   }
 }
 
+/// Check that types are consistent when two operands need to have the same
+/// number of vector elements.
+/// \return true if the types are valid.
+bool MachineVerifier::verifyVectorElementMatch(LLT Ty0, LLT Ty1,
+                                               const MachineInstr *MI) {
+  if (Ty0.isVector() != Ty1.isVector()) {
+    report("operand types must be all-vector or all-scalar", MI);
+    // Generally we try to report as many issues as possible at once, but in
+    // this case it's not clear what should we be comparing the size of the
+    // scalar with: the size of the whole vector or its lane. Instead of
+    // making an arbitrary choice and emitting not so helpful message, let's
+    // avoid the extra noise and stop here.
+    return false;
+  }
+
+  if (Ty0.isVector() && Ty0.getNumElements() != Ty1.getNumElements()) {
+    report("operand types must preserve number of vector elements", MI);
+    return false;
+  }
+
+  return true;
+}
+
 void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
   if (isFunctionSelected)
     report("Unexpected generic instruction in a Selected function", MI);
 
   const MCInstrDesc &MCID = MI->getDesc();
   unsigned NumOps = MI->getNumOperands();
 
   // Check types.
void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
   case TargetOpcode::G_INTTOPTR:
   case TargetOpcode::G_PTRTOINT:
   case TargetOpcode::G_ADDRSPACE_CAST: {
     LLT DstTy = MRI->getType(MI->getOperand(0).getReg());
     LLT SrcTy = MRI->getType(MI->getOperand(1).getReg());
     if (!DstTy.isValid() || !SrcTy.isValid())
       break;
 
-    if (DstTy.isVector() != SrcTy.isVector())
-      report("pointer casts must be all-vector or all-scalar", MI);
-    else {
-      if (DstTy.isVector() ) {
-        if (DstTy.getNumElements() != SrcTy.getNumElements()) {
-          report("pointer casts must preserve number of elements", MI);
-          break;
-        }
-      }
-    }
+    verifyVectorElementMatch(DstTy, SrcTy, MI);
 
     DstTy = DstTy.getScalarType();
     SrcTy = SrcTy.getScalarType();
 
     if (MI->getOpcode() == TargetOpcode::G_INTTOPTR) {
       if (!DstTy.isPointer())
         report("inttoptr result type must be a pointer", MI);
       if (SrcTy.isPointer())
case TargetOpcode::G_FPTRUNC: {
     // instructions aren't guaranteed to have the right number of operands or
     // types attached to them at this point
     assert(MCID.getNumOperands() == 2 && "Expected 2 operands G_*{EXT,TRUNC}");
     LLT DstTy = MRI->getType(MI->getOperand(0).getReg());
     LLT SrcTy = MRI->getType(MI->getOperand(1).getReg());
     if (!DstTy.isValid() || !SrcTy.isValid())
       break;
 
-    LLT DstElTy = DstTy.isVector() ? DstTy.getElementType() : DstTy;
-    LLT SrcElTy = SrcTy.isVector() ? SrcTy.getElementType() : SrcTy;
+    LLT DstElTy = DstTy.getScalarType();
+    LLT SrcElTy = SrcTy.getScalarType();
     if (DstElTy.isPointer() || SrcElTy.isPointer())
       report("Generic extend/truncate can not operate on pointers", MI);
 
-    if (DstTy.isVector() != SrcTy.isVector()) {
-      report("Generic extend/truncate must be all-vector or all-scalar", MI);
-      // Generally we try to report as many issues as possible at once, but in
-      // this case it's not clear what should we be comparing the size of the
-      // scalar with: the size of the whole vector or its lane. Instead of
-      // making an arbitrary choice and emitting not so helpful message, let's
-      // avoid the extra noise and stop here.
-      break;
-    }
-    if (DstTy.isVector() && DstTy.getNumElements() != SrcTy.getNumElements())
-      report("Generic vector extend/truncate must preserve number of lanes",
-             MI);
+    verifyVectorElementMatch(DstTy, SrcTy, MI);
+
     unsigned DstSize = DstElTy.getSizeInBits();
     unsigned SrcSize = SrcElTy.getSizeInBits();
     switch (MI->getOpcode()) {
     default:
       if (DstSize <= SrcSize)
         report("Generic extend has destination type no larger than source", MI);
       break;
     case TargetOpcode::G_TRUNC:
     case TargetOpcode::G_FPTRUNC:
       if (DstSize >= SrcSize)
         report("Generic truncate has destination type no smaller than source",
                MI);
       break;
     }
     break;
   }
+  case TargetOpcode::G_SELECT: {
+    LLT SelTy = MRI->getType(MI->getOperand(0).getReg());
+    LLT CondTy = MRI->getType(MI->getOperand(1).getReg());
+    if (!SelTy.isValid() || !CondTy.isValid())
+      break;
+
+    // Scalar condition select on a vector is valid.
+    if (CondTy.isVector())
+      verifyVectorElementMatch(SelTy, CondTy, MI);
+    break;
+  }
   case TargetOpcode::G_MERGE_VALUES: {
     // G_MERGE_VALUES should only be used to merge scalars into a larger scalar,
     // e.g. s2N = MERGE sN, sN
     // Merging multiple scalars into a vector is not allowed, should use
     // G_BUILD_VECTOR for that.
     LLT DstTy = MRI->getType(MI->getOperand(0).getReg());
     LLT SrcTy = MRI->getType(MI->getOperand(1).getReg());
     if (DstTy.isVector() || SrcTy.isVector())

test/CodeGen/AArch64/GlobalISel/legalize-inttoptr-xfail-2.mir

 # Namely, it checked that type index 0 is either s64 or p0 and
 # implicitly declared any type for type index 1 as legal (as soon as
 # its size is 64 bits). As LegalizerInfo::verify asserts on such a
 # definition due to type index 1 not being covered by a specific
 # action (not just `unsupportedIf`) it forces to review the definition
 # and fix the mistake: check that type index 0 is p0 and type index 1
 # is s64.
 
-# CHECK: Bad machine code: pointer casts must be all-vector or all-scalar
+# CHECK: Bad machine code: operand types must be all-vector or all-scalar
 # CHECK: LLVM ERROR: Found 1 machine code errors.
 
 ---
 name:            broken
 alignment:       2
 tracksRegLiveness: true
 registers:
   - { id: 0, class: _ }

test/CodeGen/X86/verifier-generic-extend-truncate.mir

 # RUN: not llc -o - %s -mtriple=x86_64-- -verify-machineinstrs -run-pass=none 2>&1 | FileCheck %s
 
 # CHECK: Bad machine code: Generic extend/truncate can not operate on pointers
 # CHECK-NEXT: - function:    bad_generic_extends_and_truncates
 # CHECK-NEXT: - basic block: %bb.1
 # CHECK-NEXT: - instruction: %t_p:_(s32) = G_TRUNC %p:_(p0)
 
-# CHECK: Bad machine code: Generic extend/truncate must be all-vector or all-scalar
+# CHECK: Bad machine code: operand types must be all-vector or all-scalar
 # CHECK-NEXT: - function:    bad_generic_extends_and_truncates
 # CHECK-NEXT: - basic block: %bb.2
 # CHECK-NEXT: - instruction: %se_i32:_(<2 x s64>) = G_SEXT %i32:_(s32)
 
-# CHECK: Bad machine code: Generic vector extend/truncate must preserve number of lanes
+# CHECK: Bad machine code: operand types must preserve number of vector elements
 # CHECK-NEXT: - function:    bad_generic_extends_and_truncates
 # CHECK-NEXT: - basic block: %bb.3
 # CHECK-NEXT: - instruction: %ze_v2i32:_(<4 x s64>) = G_ZEXT %v2i32:_(<2 x s32>)
 
 # CHECK: Bad machine code: Generic extend has destination type no larger than source
 # CHECK-NEXT: - function:    bad_generic_extends_and_truncates
 # CHECK-NEXT: - basic block: %bb.4
 # CHECK-NEXT: - instruction: %ae_i32:_(s32) = G_ANYEXT %i32:_(s32)
 
 # CHECK: Bad machine code: Generic truncate has destination type no smaller than source ***
 # CHECK-NEXT: - function:    bad_generic_extends_and_truncates
 # CHECK-NEXT: - basic block: %bb.5
 # CHECK-NEXT: - instruction: %ft_f32:_(s64) = G_FPTRUNC %f32:_(s32)
 
 
 # CHECK: Bad machine code: Generic extend/truncate can not operate on pointers
 # CHECK-NEXT: - function:    bad_generic_extends_and_truncates
 # CHECK-NEXT: - basic block: %bb.6
 # CHECK-NEXT: - instruction: %ze_v2i128:_(<4 x p0>) = G_ZEXT %v2i128:_(<2 x s128>)
 
-# CHECK: Bad machine code: Generic vector extend/truncate must preserve number of lanes
+# CHECK: Bad machine code: operand types must preserve number of vector elements
 # CHECK-NEXT: - function:    bad_generic_extends_and_truncates
 # CHECK-NEXT: - basic block: %bb.6
 # CHECK-NEXT: - instruction: %ze_v2i128:_(<4 x p0>) = G_ZEXT %v2i128:_(<2 x s128>)
 
 # CHECK: Bad machine code: Generic extend has destination type no larger than source
 # CHECK-NEXT: - function:    bad_generic_extends_and_truncates
 # CHECK-NEXT: - basic block: %bb.6
 # CHECK-NEXT: - instruction: %ze_v2i128:_(<4 x p0>) = G_ZEXT %v2i128:_(<2 x s128>)
 
 
 # CHECK: Bad machine code: Generic extend/truncate can not operate on pointers
 # CHECK-NEXT: - function:    bad_generic_extends_and_truncates
 # CHECK-NEXT: - basic block: %bb.6
 # CHECK-NEXT: - instruction: %fe_v2f128:_(p0) = G_FPEXT %v2f128:_(<2 x s128>)
 
-# CHECK: Bad machine code: Generic extend/truncate must be all-vector or all-scalar
+# CHECK: Bad machine code: operand types must be all-vector or all-scalar
 # CHECK-NEXT: - function:    bad_generic_extends_and_truncates
 # CHECK-NEXT: - basic block: %bb.6
 # CHECK-NEXT: - instruction: %fe_v2f128:_(p0) = G_FPEXT %v2f128:_(<2 x s128>)
 
 ---
 name:              bad_generic_extends_and_truncates
 tracksRegLiveness: true
 body:              |

test/Verifier/test_g_addrspacecast.mir

bb.0:
     %6:_(s64) = G_ADDRSPACE_CAST %1
 
     ; CHECK: Bad machine code: addrspacecast types must be pointers
     %7:_(<2 x s64>) = G_ADDRSPACE_CAST %1
 
     ; CHECK: Bad machine code: addrspacecast types must be pointers
     %8:_(<2 x p0>) = G_ADDRSPACE_CAST %2
 
-    ; CHECK: Bad machine code: pointer casts must be all-vector or all-scalar
+    ; CHECK: Bad machine code: operand types must be all-vector or all-scalar
     %9:_(<2 x p1>) = G_ADDRSPACE_CAST %1
 
-    ; CHECK: Bad machine code: pointer casts must be all-vector or all-scalar
+    ; CHECK: Bad machine code: operand types must be all-vector or all-scalar
     %10:_(p1) = G_ADDRSPACE_CAST %3
 
-    ; CHECK: Bad machine code: pointer casts must preserve number of elements
+    ; CHECK: Bad machine code: operand types must preserve number of vector elements
     %11:_(<4 x p1>) = G_ADDRSPACE_CAST %3
 
-    ; CHECK: Bad machine code: pointer casts must preserve number of elements
+    ; CHECK: Bad machine code: operand types must preserve number of vector elements
     %12:_(<4 x p1>) = G_IMPLICIT_DEF
     %13:_(<2 x p0>) = G_ADDRSPACE_CAST %12
 
     ; CHECK: Bad machine code: addrspacecast must convert different address spaces
     %14:_(p0) = G_ADDRSPACE_CAST %1
 
     ; CHECK: Bad machine code: addrspacecast must convert different address spaces
     %15:_(<2 x p0>) = G_ADDRSPACE_CAST %3
 
 ...

test/Verifier/test_g_inttoptr.mir

bb.0:
     G_INTTOPTR %0
 
     ; CHECK: Bad machine code: inttoptr result type must be a pointer
     %5:_(s64) = G_INTTOPTR %0
 
     ; CHECK: Bad machine code: inttoptr result type must be a pointer
     %6:_(<2 x s64>) = G_INTTOPTR %2
 
-    ; CHECK: Bad machine code: pointer casts must be all-vector or all-scalar
+    ; CHECK: Bad machine code: operand types must be all-vector or all-scalar
     %7:_(<2 x p0>) = G_INTTOPTR %0
 
-    ; CHECK: Bad machine code: pointer casts must be all-vector or all-scalar
+    ; CHECK: Bad machine code: operand types must be all-vector or all-scalar
     %8:_(p0) = G_INTTOPTR %2
 
-    ; CHECK: Bad machine code: pointer casts must preserve number of elements
+    ; CHECK: Bad machine code: operand types must preserve number of vector elements
     %9:_(<4 x p0>) = G_INTTOPTR %2
 
-    ; CHECK: Bad machine code: pointer casts must preserve number of elements
+    ; CHECK: Bad machine code: operand types must preserve number of vector elements
     %10:_(<4 x s64>) = G_IMPLICIT_DEF
     %11:_(<2 x p0>) = G_INTTOPTR %10
 
 ...

test/Verifier/test_g_ptrtoint.mir

bb.0:
     %5:_(p0) = G_PTRTOINT %1
 
     ; CHECK: Bad machine code: ptrtoint result type must not be a pointer
     %6:_(<2 x p0>) = G_PTRTOINT %0
 
     ; CHECK: Bad machine code: ptrtoint source type must be a pointer
     %7:_(<2 x s64>) = G_PTRTOINT %2
 
-    ; CHECK: Bad machine code: pointer casts must be all-vector or all-scalar
+    ; CHECK: Bad machine code: operand types must be all-vector or all-scalar
     %8:_(s64) = G_PTRTOINT %3
 
-    ; CHECK: Bad machine code: pointer casts must preserve number of elements
+    ; CHECK: Bad machine code: operand types must preserve number of vector elements
     %9:_(<4 x s64>) = G_INTTOPTR %3
 
-    ; CHECK: Bad machine code: pointer casts must preserve number of elements
+    ; CHECK: Bad machine code: operand types must preserve number of vector elements
     %10:_(<4 x p0>) = G_IMPLICIT_DEF
     %11:_(<2 x s64>) = G_PTRTOINT %10
 
 ...

test/Verifier/test_g_select.mir

+#RUN: not llc -march=aarch64 -run-pass=none -verify-machineinstrs -o /dev/null %s 2>&1 | FileCheck %s
+# REQUIRES: global-isel, aarch64-registered-target
+
+---
+name:            test_select
+legalized:       true
+regBankSelected: false
+selected:        false
+tracksRegLiveness: true
+liveins:
+body:             |
+  bb.0:
+
+    %0:_(s32) = G_CONSTANT i32 0
+    %1:_(s32) = G_CONSTANT i32 1
+    %2:_(s1) = G_CONSTANT i32 0
+    %3:_(<2 x s32>) = G_IMPLICIT_DEF
+    %4:_(<4 x s32>) = G_IMPLICIT_DEF
+    %5:_(<2 x s1>) = G_IMPLICIT_DEF
+    %6:_(<4 x s1>) = G_IMPLICIT_DEF
+
+    ; CHECK: Bad machine code: operand types must be all-vector or all-scalar
+    %7:_(s32) = G_SELECT %5, %0, %1
+
+    ; CHECK: Bad machine code: operand types must preserve number of vector elements
+    %8:_(<2 x s32>) = G_SELECT %6, %3, %3
+
+    ; CHECK: Bad machine code: operand types must preserve number of vector elements
+    %9:_(<4 x s32>) = G_SELECT %5, %4, %4
+
+...