[llvm] r302228 - Remove stale live-ins in the branch folder
Krzysztof Parzyszek via llvm-commits
llvm-commits at lists.llvm.org
Fri May 5 05:20:08 PDT 2017
Author: kparzysz
Date: Fri May 5 07:20:07 2017
New Revision: 302228
URL: http://llvm.org/viewvc/llvm-project?rev=302228&view=rev
Log:
Remove stale live-ins in the branch folder
Hoisting common code can cause registers that live-in in the successor
blocks to no longer be live-in. The live-in information needs to be
updated to reflect this, or otherwise incorrect code can be generated
later on.
Differential Revision: https://reviews.llvm.org/D32661
Added:
llvm/trunk/test/CodeGen/Hexagon/branch-folder-hoist-kills.mir
Modified:
llvm/trunk/lib/CodeGen/BranchFolding.cpp
Modified: llvm/trunk/lib/CodeGen/BranchFolding.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/BranchFolding.cpp?rev=302228&r1=302227&r2=302228&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/BranchFolding.cpp (original)
+++ llvm/trunk/lib/CodeGen/BranchFolding.cpp Fri May 5 07:20:07 2017
@@ -1850,8 +1850,8 @@ bool BranchFolder::HoistCommonCodeInSucc
return false;
bool HasDups = false;
- SmallVector<unsigned, 4> LocalDefs;
- SmallSet<unsigned, 4> LocalDefsSet;
+ SmallVector<unsigned, 4> LocalDefs, LocalKills;
+ SmallSet<unsigned, 4> ActiveDefsSet, AllDefsSet;
MachineBasicBlock::iterator TIB = TBB->begin();
MachineBasicBlock::iterator FIB = FBB->begin();
MachineBasicBlock::iterator TIE = TBB->end();
@@ -1905,7 +1905,7 @@ bool BranchFolder::HoistCommonCodeInSucc
IsSafe = false;
break;
}
- } else if (!LocalDefsSet.count(Reg)) {
+ } else if (!ActiveDefsSet.count(Reg)) {
if (Defs.count(Reg)) {
// Use is defined by the instruction at the point of insertion.
IsSafe = false;
@@ -1925,18 +1925,22 @@ bool BranchFolder::HoistCommonCodeInSucc
if (!TIB->isSafeToMove(nullptr, DontMoveAcrossStore))
break;
- // Remove kills from LocalDefsSet, these registers had short live ranges.
+ // Remove kills from ActiveDefsSet, these registers had short live ranges.
for (const MachineOperand &MO : TIB->operands()) {
if (!MO.isReg() || !MO.isUse() || !MO.isKill())
continue;
unsigned Reg = MO.getReg();
- if (!Reg || !LocalDefsSet.count(Reg))
+ if (!Reg)
+ continue;
+ if (!AllDefsSet.count(Reg)) {
+ LocalKills.push_back(Reg);
continue;
+ }
if (TargetRegisterInfo::isPhysicalRegister(Reg)) {
for (MCRegAliasIterator AI(Reg, TRI, true); AI.isValid(); ++AI)
- LocalDefsSet.erase(*AI);
+ ActiveDefsSet.erase(*AI);
} else {
- LocalDefsSet.erase(Reg);
+ ActiveDefsSet.erase(Reg);
}
}
@@ -1948,7 +1952,8 @@ bool BranchFolder::HoistCommonCodeInSucc
if (!Reg || TargetRegisterInfo::isVirtualRegister(Reg))
continue;
LocalDefs.push_back(Reg);
- addRegAndItsAliases(Reg, TRI, LocalDefsSet);
+ addRegAndItsAliases(Reg, TRI, ActiveDefsSet);
+ addRegAndItsAliases(Reg, TRI, AllDefsSet);
}
HasDups = true;
@@ -1963,17 +1968,22 @@ bool BranchFolder::HoistCommonCodeInSucc
FBB->erase(FBB->begin(), FIB);
// Update livein's.
- bool AddedLiveIns = false;
+ bool ChangedLiveIns = false;
for (unsigned i = 0, e = LocalDefs.size(); i != e; ++i) {
unsigned Def = LocalDefs[i];
- if (LocalDefsSet.count(Def)) {
+ if (ActiveDefsSet.count(Def)) {
TBB->addLiveIn(Def);
FBB->addLiveIn(Def);
- AddedLiveIns = true;
+ ChangedLiveIns = true;
}
}
+ for (unsigned K : LocalKills) {
+ TBB->removeLiveIn(K);
+ FBB->removeLiveIn(K);
+ ChangedLiveIns = true;
+ }
- if (AddedLiveIns) {
+ if (ChangedLiveIns) {
TBB->sortUniqueLiveIns();
FBB->sortUniqueLiveIns();
}
Added: llvm/trunk/test/CodeGen/Hexagon/branch-folder-hoist-kills.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Hexagon/branch-folder-hoist-kills.mir?rev=302228&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/Hexagon/branch-folder-hoist-kills.mir (added)
+++ llvm/trunk/test/CodeGen/Hexagon/branch-folder-hoist-kills.mir Fri May 5 07:20:07 2017
@@ -0,0 +1,59 @@
+# RUN: llc -march=hexagon -run-pass branch-folder -run-pass if-converter -verify-machineinstrs %s -o - | FileCheck %s
+
+# The hoisting of common instructions from successors could cause registers
+# to no longer be live-in in the successor blocks. The liveness was updated
+# to include potential new live-in registres, but not to remove registers
+# that were no longer live-in.
+# This could cause if-converter to generate incorrect code.
+#
+# In this testcase, the "r1 = A2_sxth r0<kill>" was hoisted, and since r0
+# was killed, it was no longer live-in in either successor. The if-converter
+# then created code, where the first predicated instruction has incorrect
+# implicit use of r0:
+#
+# BB#0:
+# Live Ins: %R0
+# %R1<def> = A2_sxth %R0<kill> ; hoisted, kills r0
+# A2_nop %P0<imp-def>
+# %R0<def> = C2_cmoveit %P0, 2, %R0<imp-use> ; predicated A2_tfrsi
+# %R0<def> = C2_cmoveif %P0, 1, %R0<imp-use> ; predicated A2_tfrsi
+# %R0<def> = A2_add %R0<kill>, %R1<kill>
+# J2_jumpr %R31, %PC<imp-def,dead>
+#
+
+# CHECK: %r1 = A2_sxth killed %r0
+# CHECK: %r0 = C2_cmoveit %p0, 2
+# CHECK-NOT: implicit-def %r0
+# CHECK: %r0 = C2_cmoveif %p0, 1, implicit %r0
+
+---
+name: fred
+tracksRegLiveness: true
+
+body: |
+ bb.0:
+ liveins: %r0
+ successors: %bb.1, %bb.2
+
+ A2_nop implicit-def %p0
+ J2_jumpt killed %p0, %bb.2, implicit-def dead %pc
+
+ bb.1:
+ successors: %bb.3
+ liveins: %r0
+ %r1 = A2_sxth killed %r0
+ %r0 = A2_tfrsi 1
+ J2_jump %bb.3, implicit-def %pc
+
+ bb.2:
+ successors: %bb.3
+ liveins: %r0
+ %r1 = A2_sxth killed %r0
+ %r0 = A2_tfrsi 2
+
+ bb.3:
+ liveins: %r0, %r1
+ %r0 = A2_add killed %r0, killed %r1
+ J2_jumpr %r31, implicit-def dead %pc
+...
+
More information about the llvm-commits
mailing list