From b2154af25f52589360eb43e2b74413dbba461aff Mon Sep 17 00:00:00 2001 From: Bjorn Pettersson Date: Thu, 20 Sep 2018 06:59:18 +0000 Subject: [PATCH] [MachineVerifier] Relax checkLivenessAtDef regarding dead subreg defs Summary: Consider an instruction that has multiple defs of the same vreg, but defining different subregs: %7.sub1:rc, dead %7.sub2:rc = inst Calling checkLivenessAtDef for the live interval associated with %7 incorrectly reported "live range continues after a dead def". The live range for %7 has a dead def at the slot index for "inst" even if the live range continues (given that there are later uses of %7.sub1). This patch adjusts MachineVerifier::checkLivenessAtDef to allow dead subregister definitions, unless we are checking a subrange (when tracking subregister liveness). A limitation is that we do not detect the situation when the live range continues past an instruction that defines the full virtual register by multiple dead subreg defines. I also removed some dead code related to physical register in checkLivenessAtDef. Wwe only call that method for virtual registers, so I added an assertion instead. Reviewers: kparzysz Reviewed By: kparzysz Subscribers: llvm-commits Differential Revision: https://reviews.llvm.org/D52237 llvm-svn: 342618 --- llvm/lib/CodeGen/MachineVerifier.cpp | 32 ++++------ .../CodeGen/Hexagon/verify-liveness-at-def.mir | 74 ++++++++++++++++++++++ 2 files changed, 85 insertions(+), 21 deletions(-) create mode 100644 llvm/test/CodeGen/Hexagon/verify-liveness-at-def.mir diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp index 8b27470..aebfb08 100644 --- a/llvm/lib/CodeGen/MachineVerifier.cpp +++ b/llvm/lib/CodeGen/MachineVerifier.cpp @@ -262,6 +262,7 @@ namespace { LaneBitmask LaneMask = LaneBitmask::getNone()); void checkLivenessAtDef(const MachineOperand *MO, unsigned MONum, SlotIndex DefIdx, const LiveRange &LR, unsigned VRegOrUnit, + bool SubRangeCheck = false, LaneBitmask LaneMask = LaneBitmask::getNone()); void markReachable(const MachineBasicBlock *MBB); @@ -1396,7 +1397,7 @@ void MachineVerifier::checkLivenessAtUse(const MachineOperand *MO, void MachineVerifier::checkLivenessAtDef(const MachineOperand *MO, unsigned MONum, SlotIndex DefIdx, const LiveRange &LR, unsigned VRegOrUnit, - LaneBitmask LaneMask) { + bool SubRangeCheck, LaneBitmask LaneMask) { if (const VNInfo *VNI = LR.getVNInfoAt(DefIdx)) { assert(VNI && "NULL valno is not allowed"); if (VNI->def != DefIdx) { @@ -1420,25 +1421,14 @@ void MachineVerifier::checkLivenessAtDef(const MachineOperand *MO, if (MO->isDead()) { LiveQueryResult LRQ = LR.Query(DefIdx); if (!LRQ.isDeadDef()) { - // In case of physregs we can have a non-dead definition on another - // operand. - bool otherDef = false; - if (!TargetRegisterInfo::isVirtualRegister(VRegOrUnit)) { - const MachineInstr &MI = *MO->getParent(); - for (const MachineOperand &MO : MI.operands()) { - if (!MO.isReg() || !MO.isDef() || MO.isDead()) - continue; - unsigned Reg = MO.getReg(); - for (MCRegUnitIterator Units(Reg, TRI); Units.isValid(); ++Units) { - if (*Units == VRegOrUnit) { - otherDef = true; - break; - } - } - } - } - - if (!otherDef) { + assert(TargetRegisterInfo::isVirtualRegister(VRegOrUnit) && + "Expecting a virtual register."); + // A dead subreg def only tells us that the specific subreg is dead. There + // could be other non-dead defs of other subregs, or we could have other + // parts of the register being live through the instruction. So unless we + // are checking liveness for a subrange it is ok for the live range to + // continue, given that we have a dead def of a subregister. + if (SubRangeCheck || MO->getSubReg() == 0) { report("Live range continues after dead def flag", MO, MONum); report_context_liverange(LR); report_context_vreg_regunit(VRegOrUnit); @@ -1596,7 +1586,7 @@ void MachineVerifier::checkLiveness(const MachineOperand *MO, unsigned MONum) { for (const LiveInterval::SubRange &SR : LI.subranges()) { if ((SR.LaneMask & MOMask).none()) continue; - checkLivenessAtDef(MO, MONum, DefIdx, SR, Reg, SR.LaneMask); + checkLivenessAtDef(MO, MONum, DefIdx, SR, Reg, true, SR.LaneMask); } } } else { diff --git a/llvm/test/CodeGen/Hexagon/verify-liveness-at-def.mir b/llvm/test/CodeGen/Hexagon/verify-liveness-at-def.mir new file mode 100644 index 0000000..149790c --- /dev/null +++ b/llvm/test/CodeGen/Hexagon/verify-liveness-at-def.mir @@ -0,0 +1,74 @@ +# Using a trick to run simple-register-coalescing twice, that way +# liveintervals should be preserved while running the machine verifier. +# +# RUN: not llc -o - %s -march=hexagon -hexagon-subreg-liveness=false -run-pass simple-register-coalescing -verify-machineinstrs -run-pass simple-register-coalescing 2>&1 | FileCheck -check-prefix=CHECK-NOSUB %s +# RUN: not llc -o - %s -march=hexagon -hexagon-subreg-liveness=true -run-pass simple-register-coalescing -verify-machineinstrs -run-pass simple-register-coalescing 2>&1 | FileCheck -check-prefix=CHECK-SUB %s + +--- +name: test_pass +tracksRegLiveness: true +body: | + bb.0: + A2_nop implicit-def %0:doubleregs + A2_nop implicit-def dead %0.isub_lo, implicit-def %0.isub_hi, implicit %0 + A2_nop implicit %0.isub_hi +... + +--- +name: test_fail +tracksRegLiveness: true +body: | + bb.0: + A2_nop implicit-def %0:doubleregs + A2_nop implicit-def dead %0.isub_lo, implicit-def %0.isub_hi, implicit %0 + A2_nop implicit %0.isub_lo + + A2_nop implicit-def %1:doubleregs + A2_nop implicit-def dead %1.isub_lo, implicit-def dead %1.isub_hi, implicit %1 + A2_nop implicit %1 + + A2_nop implicit-def dead %2:doubleregs + A2_nop implicit %2 + +... + +############################################################################### +# We are expecting four "Bad machine code" when subregister liveness is used. +# +# CHECK-SUB-NOT: Bad machine code +# +# CHECK-SUB: Bad machine code: Live range continues after dead def flag +# CHECK_SUB-NEXT: function: test_fail +# CHECK-SUB: v. register: %0 +# CHECK-SUB: lanemask: 00000002 +# +# CHECK-SUB-NOT: Bad machine code +# +# CHECK-SUB: Bad machine code: Live range continues after dead def flag +# CHECK-SUB-NEXT: function: test_fail +# CHECK-SUB: v. register: %1 +# CHECK-SUB: lanemask: 00000002 +# +# CHECK-SUB-NOT: Bad machine code +# +# CHECK-SUB: Bad machine code: Live range continues after dead def flag +# CHECK-SUB-NEXT: function: test_fail +# CHECK-SUB: v. register: %1 +# CHECK-SUB: lanemask: 00000001 +# +# CHECK-SUB: Bad machine code: Live range continues after dead def flag +# CHECK-SUB-NEXT: function: test_fail +# CHECK: v. register: %2 +# +# CHECK-SUB-NOT: Bad machine code + +############################################################################### +# Without subregister liveness we only detect one of the failing scenarios. +# +# CHECK-NOSUB-NOT: Bad machine code +# +# CHECK-NOSUB: Bad machine code: Live range continues after dead def flag +# CHECK-NOSUB-NEXT: function: test_fail +# CHECK: v. register: %2 +# +# CHECK-NOSUB-NOT: Bad machine code -- 2.7.4