[llvm] r236660 - Handle dead defs in the if converter.
Pete Cooper
peter_cooper at apple.com
Wed May 6 15:51:04 PDT 2015
Author: pete
Date: Wed May 6 17:51:04 2015
New Revision: 236660
URL: http://llvm.org/viewvc/llvm-project?rev=236660&view=rev
Log:
Handle dead defs in the if converter.
We had code such as this:
r2 = ...
t2Bcc
label1:
ldr ... r2
label2;
return r2<dead, def>
The if converter was transforming this to
r2<def> = ...
return [pred] r2<dead,def>
ldr <r2, kill>
return
which fails the machine verifier because the ldr now reads from a dead def.
The fix here detects dead defs in stepForward and passes them back to the caller in the clobbers list. The caller then clears the dead flag from the def is the value is live.
Added:
llvm/trunk/test/CodeGen/ARM/ifcvt-dead-def.ll
Modified:
llvm/trunk/lib/CodeGen/IfConversion.cpp
llvm/trunk/lib/CodeGen/LivePhysRegs.cpp
Modified: llvm/trunk/lib/CodeGen/IfConversion.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/IfConversion.cpp?rev=236660&r1=236659&r2=236660&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/IfConversion.cpp (original)
+++ llvm/trunk/lib/CodeGen/IfConversion.cpp Wed May 6 17:51:04 2015
@@ -980,10 +980,10 @@ static void UpdatePredRedefs(MachineInst
// Now add the implicit uses for each of the clobbered values.
for (auto Reg : Clobbers) {
- const MachineOperand &Op = *Reg.second;
// FIXME: Const cast here is nasty, but better than making StepForward
// take a mutable instruction instead of const.
- MachineInstr *OpMI = const_cast<MachineInstr*>(Op.getParent());
+ MachineOperand &Op = const_cast<MachineOperand&>(*Reg.second);
+ MachineInstr *OpMI = Op.getParent();
MachineInstrBuilder MIB(*OpMI->getParent()->getParent(), OpMI);
if (Op.isRegMask()) {
// First handle regmasks. They clobber any entries in the mask which
@@ -999,6 +999,12 @@ static void UpdatePredRedefs(MachineInst
continue;
}
assert(Op.isReg() && "Register operand required");
+ if (Op.isDead()) {
+ // If we found a dead def, but it needs to be live, then remove the dead
+ // flag.
+ if (Redefs.contains(Op.getReg()))
+ Op.setIsDead(false);
+ }
MIB.addReg(Reg.first, RegState::Implicit | RegState::Undef);
}
}
Modified: llvm/trunk/lib/CodeGen/LivePhysRegs.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/LivePhysRegs.cpp?rev=236660&r1=236659&r2=236660&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/LivePhysRegs.cpp (original)
+++ llvm/trunk/lib/CodeGen/LivePhysRegs.cpp Wed May 6 17:51:04 2015
@@ -77,8 +77,9 @@ void LivePhysRegs::stepForward(const Mac
if (Reg == 0)
continue;
if (O->isDef()) {
- if (!O->isDead())
- Clobbers.push_back(std::make_pair(Reg, &*O));
+ // Note, dead defs are still recorded. The caller should decide how to
+ // handle them.
+ Clobbers.push_back(std::make_pair(Reg, &*O));
} else {
if (!O->isKill())
continue;
@@ -90,8 +91,12 @@ void LivePhysRegs::stepForward(const Mac
}
// Add defs to the set.
- for (auto Reg : Clobbers)
+ for (auto Reg : Clobbers) {
+ // Skip dead defs. They shouldn't be added to the set.
+ if (Reg.second->isReg() && Reg.second->isDead())
+ continue;
addReg(Reg.first);
+ }
}
/// Prin the currently live registers to OS.
Added: llvm/trunk/test/CodeGen/ARM/ifcvt-dead-def.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/ifcvt-dead-def.ll?rev=236660&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/ARM/ifcvt-dead-def.ll (added)
+++ llvm/trunk/test/CodeGen/ARM/ifcvt-dead-def.ll Wed May 6 17:51:04 2015
@@ -0,0 +1,55 @@
+; RUN: llc %s -o - -verify-machineinstrs | FileCheck %s
+
+target datalayout = "e-m:o-p:32:32-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32"
+target triple = "thumbv7-unknown-unknown"
+
+%struct.ref_s = type { %union.v, i16, i16 }
+%union.v = type { i32 }
+%struct.gs_color_s = type { i16, i16, i16, i16, i8, i8 }
+
+; In this case, the if converter was cloning the return instruction so that we had
+; r2<def> = ...
+; return [pred] r2<dead,def>
+; ldr <r2, kill>
+; return
+; The problem here was that the dead def on the first return was making the machine verifier
+; think that the load read from an undefined register. We need to remove the dead flag from
+; the return, and also add an implicit use of the prior value of r2.
+
+; CHECK: ldrh
+
+; Function Attrs: minsize nounwind optsize ssp
+define i32 @test(%struct.ref_s* %pref1, %struct.ref_s* %pref2, %struct.gs_color_s** %tmp152) #0 {
+bb:
+ %nref = alloca %struct.ref_s, align 4
+ %tmp46 = call %struct.ref_s* @name_string_ref(%struct.ref_s* %pref1, %struct.ref_s* %nref) #2
+ %tmp153 = load %struct.gs_color_s*, %struct.gs_color_s** %tmp152, align 4
+ %tmp154 = bitcast %struct.ref_s* %pref2 to %struct.gs_color_s**
+ %tmp155 = load %struct.gs_color_s*, %struct.gs_color_s** %tmp154, align 4
+ %tmp162 = getelementptr inbounds %struct.gs_color_s, %struct.gs_color_s* %tmp153, i32 0, i32 1
+ %tmp163 = load i16, i16* %tmp162, align 2
+ %tmp164 = getelementptr inbounds %struct.gs_color_s, %struct.gs_color_s* %tmp155, i32 0, i32 1
+ %tmp165 = load i16, i16* %tmp164, align 2
+ %tmp166 = icmp eq i16 %tmp163, %tmp165
+ br i1 %tmp166, label %bb167, label %bb173
+
+bb167: ; preds = %bb
+ %tmp168 = getelementptr inbounds %struct.gs_color_s, %struct.gs_color_s* %tmp153, i32 0, i32 2
+ %tmp169 = load i16, i16* %tmp168, align 2
+ %tmp170 = getelementptr inbounds %struct.gs_color_s, %struct.gs_color_s* %tmp155, i32 0, i32 2
+ %tmp171 = load i16, i16* %tmp170, align 2
+ %tmp172 = icmp eq i16 %tmp169, %tmp171
+ br label %bb173
+
+bb173: ; preds = %bb167, %bb
+ %tmp174 = phi i1 [ false, %bb ], [ %tmp172, %bb167 ]
+ %tmp175 = zext i1 %tmp174 to i32
+ ret i32 %tmp175
+}
+
+; Function Attrs: minsize optsize
+declare %struct.ref_s* @name_string_ref(%struct.ref_s*, %struct.ref_s*) #1
+
+attributes #0 = { minsize nounwind optsize }
+attributes #1 = { minsize optsize }
+attributes #2 = { minsize nounwind optsize }
More information about the llvm-commits
mailing list