[llvm] r310979 - [VirtRegRewriter] Properly model the register liveness on undef subreg definition
Quentin Colombet via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 15 17:17:05 PDT 2017
Author: qcolombet
Date: Tue Aug 15 17:17:05 2017
New Revision: 310979
URL: http://llvm.org/viewvc/llvm-project?rev=310979&view=rev
Log:
[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<def,read-undef> = COPY %R0 ; <-- R1 is still live here
%vreg14:gsub_1<def> = COPY %R1
Before this changes, the rewriter would change the code into:
%R0<def> = KILL %R0, %R0_R1<imp-def> ; <-- this tells R1 is redefined
%R1<def> = KILL %R1, %R0_R1<imp-def>, %R0_R1<imp-use> ; 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<def> = KILL %R0, %R0_R1<imp-def>, %R0_R1<imp-use> ; This tells R1 needs to
; reach this point
%R1<def> = KILL %R1, %R0_R1<imp-def>, %R0_R1<imp-use>
The bug has been here forever, it got exposed recently because the register
scavenger got smarter.
Fixes llvm.org/PR34107
Added:
llvm/trunk/test/CodeGen/ARM/virtregrewriter-subregliveness.mir
Modified:
llvm/trunk/lib/CodeGen/VirtRegMap.cpp
Modified: llvm/trunk/lib/CodeGen/VirtRegMap.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/VirtRegMap.cpp?rev=310979&r1=310978&r2=310979&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/VirtRegMap.cpp (original)
+++ llvm/trunk/lib/CodeGen/VirtRegMap.cpp Tue Aug 15 17:17:05 2017
@@ -180,6 +180,7 @@ class VirtRegRewriter : public MachineFu
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(M
}
}
+/// 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<unsigned, 8> SuperDeads;
@@ -452,7 +479,8 @@ void VirtRegRewriter::rewrite() {
// A virtual register kill refers to the whole register, so we may
// have to add <imp-use,kill> 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()) {
Added: llvm/trunk/test/CodeGen/ARM/virtregrewriter-subregliveness.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/virtregrewriter-subregliveness.mir?rev=310979&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/ARM/virtregrewriter-subregliveness.mir (added)
+++ llvm/trunk/test/CodeGen/ARM/virtregrewriter-subregliveness.mir Tue Aug 15 17:17:05 2017
@@ -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
+
+
+...
More information about the llvm-commits
mailing list