From dc77820944840c165047cd8d81fb17f7ffe3c004 Mon Sep 17 00:00:00 2001 From: Roman Tereshin Date: Tue, 8 May 2018 02:48:15 +0000 Subject: [PATCH] [MachineVerifier][GlobalISel] Verifying generic extends and truncates Making sure we don't truncate / extend pointers, don't try to change vector topology or bitcast vectors to scalars or back, and most importantly, don't extend to a smaller type or truncate to a large one. Reviewers: qcolombet t.p.northover aditya_nandakumar Reviewed By: qcolombet Subscribers: rovka, kristof.beyls, llvm-commits Differential Revision: https://reviews.llvm.org/D46490 llvm-svn: 331718 --- lib/CodeGen/MachineVerifier.cpp | 52 +++++++++++ .../AMDGPU/GlobalISel/regbankselect-trunc.mir | 12 +-- .../X86/verifier-generic-extend-truncate.mir | 87 +++++++++++++++++++ 3 files changed, 145 insertions(+), 6 deletions(-) create mode 100644 test/CodeGen/X86/verifier-generic-extend-truncate.mir diff --git a/lib/CodeGen/MachineVerifier.cpp b/lib/CodeGen/MachineVerifier.cpp index ab5b389a111..8d518571d24 100644 --- a/lib/CodeGen/MachineVerifier.cpp +++ b/lib/CodeGen/MachineVerifier.cpp @@ -986,6 +986,58 @@ void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) { MI); break; } + case TargetOpcode::G_SEXT: + case TargetOpcode::G_ZEXT: + case TargetOpcode::G_ANYEXT: + case TargetOpcode::G_TRUNC: + case TargetOpcode::G_FPEXT: + case TargetOpcode::G_FPTRUNC: { + // Number of operands and presense of types is already checked (and + // reported in case of any issues), so no need to report them again. As + // we're trying to report as many issues as possible at once, however, the + // 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}"); + if (MI->getNumOperands() < MCID.getNumOperands()) + break; + 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; + 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); + 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::COPY: { if (foundErrors) break; diff --git a/test/CodeGen/AMDGPU/GlobalISel/regbankselect-trunc.mir b/test/CodeGen/AMDGPU/GlobalISel/regbankselect-trunc.mir index 16f9624a48d..14de218554f 100644 --- a/test/CodeGen/AMDGPU/GlobalISel/regbankselect-trunc.mir +++ b/test/CodeGen/AMDGPU/GlobalISel/regbankselect-trunc.mir @@ -10,9 +10,9 @@ body: | bb.0: liveins: $sgpr0_sgpr1 ; CHECK-LABEL: name: trunc_i64_to_i32_s - ; CHECK: [[COPY:%[0-9]+]]:sgpr(s32) = COPY $sgpr0 - ; CHECK: [[TRUNC:%[0-9]+]]:sgpr(s32) = G_TRUNC [[COPY]](s32) - %0:_(s32) = COPY $sgpr0 + ; CHECK: [[COPY:%[0-9]+]]:sgpr(s64) = COPY $sgpr0 + ; CHECK: [[TRUNC:%[0-9]+]]:sgpr(s32) = G_TRUNC [[COPY]](s64) + %0:_(s64) = COPY $sgpr0_sgpr1 %1:_(s32) = G_TRUNC %0 ... @@ -24,8 +24,8 @@ body: | bb.0: liveins: $vgpr0_vgpr1 ; CHECK-LABEL: name: trunc_i64_to_i32_v - ; CHECK: [[COPY:%[0-9]+]]:vgpr(s32) = COPY $vgpr0 - ; CHECK: [[TRUNC:%[0-9]+]]:vgpr(s32) = G_TRUNC [[COPY]](s32) - %0:_(s32) = COPY $vgpr0 + ; CHECK: [[COPY:%[0-9]+]]:vgpr(s64) = COPY $vgpr0 + ; CHECK: [[TRUNC:%[0-9]+]]:vgpr(s32) = G_TRUNC [[COPY]](s64) + %0:_(s64) = COPY $vgpr0_vgpr1 %1:_(s32) = G_TRUNC %0 ... diff --git a/test/CodeGen/X86/verifier-generic-extend-truncate.mir b/test/CodeGen/X86/verifier-generic-extend-truncate.mir new file mode 100644 index 00000000000..802a59f2863 --- /dev/null +++ b/test/CodeGen/X86/verifier-generic-extend-truncate.mir @@ -0,0 +1,87 @@ +# 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-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-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-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-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: | + bb.0: + liveins: $rdi, $esi, $rdx, $xmm0, $ymm1, $ymm2 + + %p:_(p0) = COPY $rdi + %i32:_(s32) = COPY $esi + %v2i32:_(<2 x s32>) = COPY $rdx + %f32:_(s32) = COPY $xmm0 + %v2i128:_(<2 x s128>) = COPY $ymm1 + %v2f128:_(<2 x s128>) = COPY $ymm2 + + bb.1: + %t_p:_(s32) = G_TRUNC %p + + bb.2: + %se_i32:_(<2 x s64>) = G_SEXT %i32 + + bb.3: + %ze_v2i32:_(<4 x s64>) = G_ZEXT %v2i32 + + bb.4: + %ae_i32:_(s32) = G_ANYEXT %i32 + + bb.5: + %ft_f32:_(s64) = G_FPTRUNC %f32 + + bb.6: + %ze_v2i128:_(<4 x p0>) = G_ZEXT %v2i128 + %fe_v2f128:_(p0) = G_FPEXT %v2f128 +...