From f2a26339e2bdd5a13982d21d21c0bdf1d37b9ab8 Mon Sep 17 00:00:00 2001 From: Matt Arsenault Date: Mon, 4 Feb 2019 23:29:16 +0000 Subject: [PATCH] GlobalISel: Verify g_select Factor the common vector element consistency check many instructions need out, although this makes the error messages worse. llvm-svn: 353112 --- llvm/lib/CodeGen/MachineVerifier.cpp | 64 ++++++++++++++-------- .../GlobalISel/legalize-inttoptr-xfail-2.mir | 2 +- .../X86/verifier-generic-extend-truncate.mir | 8 +-- llvm/test/Verifier/test_g_addrspacecast.mir | 8 +-- llvm/test/Verifier/test_g_inttoptr.mir | 8 +-- llvm/test/Verifier/test_g_ptrtoint.mir | 6 +- llvm/test/Verifier/test_g_select.mir | 31 +++++++++++ 7 files changed, 87 insertions(+), 40 deletions(-) create mode 100644 llvm/test/Verifier/test_g_select.mir diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp index 7e5ed6a..96755d7 100644 --- a/llvm/lib/CodeGen/MachineVerifier.cpp +++ b/llvm/lib/CodeGen/MachineVerifier.cpp @@ -231,6 +231,7 @@ namespace { 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); @@ -890,6 +891,29 @@ void MachineVerifier::verifyInlineAsm(const MachineInstr *MI) { } } +/// 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); @@ -1021,16 +1045,7 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) { 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(); @@ -1074,23 +1089,13 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) { 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()) { @@ -1107,6 +1112,17 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) { } 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 diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/legalize-inttoptr-xfail-2.mir b/llvm/test/CodeGen/AArch64/GlobalISel/legalize-inttoptr-xfail-2.mir index 18fd182..3956f3a 100644 --- a/llvm/test/CodeGen/AArch64/GlobalISel/legalize-inttoptr-xfail-2.mir +++ b/llvm/test/CodeGen/AArch64/GlobalISel/legalize-inttoptr-xfail-2.mir @@ -19,7 +19,7 @@ # 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. --- diff --git a/llvm/test/CodeGen/X86/verifier-generic-extend-truncate.mir b/llvm/test/CodeGen/X86/verifier-generic-extend-truncate.mir index 802a59f..28bca88 100644 --- a/llvm/test/CodeGen/X86/verifier-generic-extend-truncate.mir +++ b/llvm/test/CodeGen/X86/verifier-generic-extend-truncate.mir @@ -5,12 +5,12 @@ # 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>) @@ -31,7 +31,7 @@ # 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>) @@ -47,7 +47,7 @@ # 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>) diff --git a/llvm/test/Verifier/test_g_addrspacecast.mir b/llvm/test/Verifier/test_g_addrspacecast.mir index ef4c6c3..4450410 100644 --- a/llvm/test/Verifier/test_g_addrspacecast.mir +++ b/llvm/test/Verifier/test_g_addrspacecast.mir @@ -35,16 +35,16 @@ body: | ; 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 diff --git a/llvm/test/Verifier/test_g_inttoptr.mir b/llvm/test/Verifier/test_g_inttoptr.mir index 02a802a..f3a2011 100644 --- a/llvm/test/Verifier/test_g_inttoptr.mir +++ b/llvm/test/Verifier/test_g_inttoptr.mir @@ -29,16 +29,16 @@ body: | ; 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 diff --git a/llvm/test/Verifier/test_g_ptrtoint.mir b/llvm/test/Verifier/test_g_ptrtoint.mir index 40e47d4..c6a969a 100644 --- a/llvm/test/Verifier/test_g_ptrtoint.mir +++ b/llvm/test/Verifier/test_g_ptrtoint.mir @@ -32,13 +32,13 @@ body: | ; 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 diff --git a/llvm/test/Verifier/test_g_select.mir b/llvm/test/Verifier/test_g_select.mir new file mode 100644 index 0000000..d40b276 --- /dev/null +++ b/llvm/test/Verifier/test_g_select.mir @@ -0,0 +1,31 @@ +#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 + +... -- 2.7.4