From 156a10346f9be4985bd8b6ceb6bfe9fd7c7a0941 Mon Sep 17 00:00:00 2001 From: Quentin Colombet Date: Wed, 16 Aug 2017 00:17:05 +0000 Subject: [PATCH] [VirtRegRewriter] Properly model the register liveness on undef subreg definition Undef subreg definition means that the content of the super register doesn't matter at this point. While that's true for virtual registers, this may not hold when replacing them with actual physical registers. Indeed, some part of the physical register may be coalesced with the related virtual register and thus, the values for those parts matter and must be live. The fix consists in checking whether or not subregs of the physical register being assigned to an undef subreg definition are live through that def and insert an implicit use if they are. Doing so, will keep them alive until that point like they should be. E.g., let vreg14 being assigned to R0_R1 then %vreg14:gsub_0 = COPY %R0 ; <-- R1 is still live here %vreg14:gsub_1 = COPY %R1 Before this changes, the rewriter would change the code into: %R0 = KILL %R0, %R0_R1 ; <-- this tells R1 is redefined %R1 = KILL %R1, %R0_R1, %R0_R1 ; this value of this R1 ; is believed to come ; from the previous ; instruction Because of this invalid liveness, later pass could make wrong choices and in particular clobber live register as it happened with the register scavenger in llvm.org/PR34107 Now we would generate: %R0 = KILL %R0, %R0_R1, %R0_R1 ; This tells R1 needs to ; reach this point %R1 = KILL %R1, %R0_R1, %R0_R1 The bug has been here forever, it got exposed recently because the register scavenger got smarter. Fixes llvm.org/PR34107 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@310979 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/VirtRegMap.cpp | 30 +++++++- .../CodeGen/ARM/virtregrewriter-subregliveness.mir | 84 ++++++++++++++++++++++ 2 files changed, 113 insertions(+), 1 deletion(-) create mode 100644 test/CodeGen/ARM/virtregrewriter-subregliveness.mir diff --git a/lib/CodeGen/VirtRegMap.cpp b/lib/CodeGen/VirtRegMap.cpp index 124c2790f68..f8aacdb8649 100644 --- a/lib/CodeGen/VirtRegMap.cpp +++ b/lib/CodeGen/VirtRegMap.cpp @@ -180,6 +180,7 @@ class VirtRegRewriter : public MachineFunctionPass { void addLiveInsForSubRanges(const LiveInterval &LI, unsigned PhysReg) const; void handleIdentityCopy(MachineInstr &MI) const; void expandCopyBundle(MachineInstr &MI) const; + bool subRegLiveThrough(const MachineInstr &MI, unsigned SuperPhysReg) const; public: static char ID; @@ -415,6 +416,32 @@ void VirtRegRewriter::expandCopyBundle(MachineInstr &MI) const { } } +/// Check whether (part of) \p SuperPhysReg is live through \p MI. +/// \pre \p MI defines a subregister of a virtual register that +/// has been assigned to \p SuperPhysReg. +bool VirtRegRewriter::subRegLiveThrough(const MachineInstr &MI, + unsigned SuperPhysReg) const { + SlotIndex MIIndex = LIS->getInstructionIndex(MI); + SlotIndex BeforeMIUses = MIIndex.getBaseIndex(); + SlotIndex AfterMIDefs = MIIndex.getBoundaryIndex(); + for (MCRegUnitIterator Unit(SuperPhysReg, TRI); Unit.isValid(); ++Unit) { + const LiveRange &UnitRange = LIS->getRegUnit(*Unit); + // If the regunit is live both before and after MI, + // we assume it is live through. + // Generally speaking, this is not true, because something like + // "RU = op RU" would match that description. + // However, we know that we are trying to assess whether + // a def of a virtual reg, vreg, is live at the same time of RU. + // If we are in the "RU = op RU" situation, that means that vreg + // is defined at the same time as RU (i.e., "vreg, RU = op RU"). + // Thus, vreg and RU interferes and vreg cannot be assigned to + // SuperPhysReg. Therefore, this situation cannot happen. + if (UnitRange.liveAt(AfterMIDefs) && UnitRange.liveAt(BeforeMIUses)) + return true; + } + return false; +} + void VirtRegRewriter::rewrite() { bool NoSubRegLiveness = !MRI->subRegLivenessEnabled(); SmallVector SuperDeads; @@ -452,7 +479,8 @@ void VirtRegRewriter::rewrite() { // A virtual register kill refers to the whole register, so we may // have to add operands for the super-register. A // partial redef always kills and redefines the super-register. - if (MO.readsReg() && (MO.isDef() || MO.isKill())) + if ((MO.readsReg() && (MO.isDef() || MO.isKill())) || + (MO.isDef() && subRegLiveThrough(*MI, PhysReg))) SuperKills.push_back(PhysReg); if (MO.isDef()) { diff --git a/test/CodeGen/ARM/virtregrewriter-subregliveness.mir b/test/CodeGen/ARM/virtregrewriter-subregliveness.mir new file mode 100644 index 00000000000..83335a3ccff --- /dev/null +++ b/test/CodeGen/ARM/virtregrewriter-subregliveness.mir @@ -0,0 +1,84 @@ +# RUN: llc -o - -mtriple=thumbv7--windows-gnu -run-pass=greedy -run-pass=virtregrewriter %s | FileCheck %s +--- | + target datalayout = "e-m:w-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64" + target triple = "thumbv7--windows-gnu" + + define void @subregLiveThrough() { ret void } + define void @subregNotLiveThrough() { ret void } + define void @subregNotLiveThrough2() { ret void } + +... +--- +# Check that we properly recognize that r1 is live through +# the first subreg copy. +# That will materialize as an implicit use of the big register +# on that copy. +# PR34107. +# +# CHECK-LABEL: name: subregLiveThrough +name: subregLiveThrough +tracksRegLiveness: true +registers: + - { id: 0, class: gprpair } +body: | + bb.0: + liveins: %r0, %r1 + + ; That copy is being coalesced so we should use a KILL + ; placeholder. If that's not a kill that means we probably + ; not coalescing %0 and %r0_r1 and thus we are not testing + ; the problematic code anymore. + ; + ; CHECK: %r0 = KILL %r0, implicit killed %r0_r1, implicit-def %r0_r1 + ; CHECK-NEXT: %r1 = KILL %r1, implicit killed %r0_r1 + undef %0.gsub_0 = COPY %r0 + %0.gsub_1 = COPY %r1 + tBX_RET 14, _, implicit %0 + + +... + +--- +# Check that we properly recognize that r1 is *not* live through +# the first subreg copy. +# CHECK-LABEL: name: subregNotLiveThrough +name: subregNotLiveThrough +tracksRegLiveness: true +registers: + - { id: 0, class: gprpair } +body: | + bb.0: + liveins: %r0, %r1 + + ; r1 is not live through so check we are not implicitly using + ; the big register. + ; CHECK: %r0 = KILL %r0, implicit-def %r0_r1 + ; CHECK-NEXT: tBX_RET + undef %0.gsub_0 = COPY %r0 + tBX_RET 14, _, implicit %0 + + +... + +--- +# Check that we properly recognize that r1 is *not* live through +# the first subreg copy. It is defined by this copy, but is not +# through. +# CHECK-LABEL: name: subregNotLiveThrough2 +name: subregNotLiveThrough2 +tracksRegLiveness: true +registers: + - { id: 0, class: gprpair } +body: | + bb.0: + liveins: %r0, %r1 + + ; r1 is not live through so check we are not implicitly using + ; the big register. + ; CHECK: %r0 = KILL %r0, implicit-def %r1, implicit-def %r0_r1 + ; CHECK-NEXT: tBX_RET + undef %0.gsub_0 = COPY %r0, implicit-def %r1 + tBX_RET 14, _, implicit %0 + + +... -- 2.11.0