Updated version of r96634 (which was reverted due to failing 176.gcc and
126.gcc nightly tests. These failures uncovered latent bugs that machine DCE
could remove one half of a stack adjust down/up pair, causing PEI to assert.
This update fixes that, and the tests now pass.
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@96822 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/llvm/Target/TargetRegisterInfo.h b/include/llvm/Target/TargetRegisterInfo.h
index 65b60f7..212cc93 100644
--- a/include/llvm/Target/TargetRegisterInfo.h
+++ b/include/llvm/Target/TargetRegisterInfo.h
@@ -587,6 +587,17 @@
return !hasFP(MF);
}
+ /// canSimplifyCallFramePseudos - When possible, it's best to simplify the
+ /// call frame pseudo ops before doing frame index elimination. This is
+ /// possible only when frame index references between the pseudos won't
+ /// need adjusted for the call frame adjustments. Normally, that's true
+ /// if the function has a reserved call frame or a frame pointer. Some
+ /// targets (Thumb2, for example) may have more complicated criteria,
+ /// however, and can override this behavior.
+ virtual bool canSimplifyCallFramePseudos(MachineFunction &MF) const {
+ return hasReservedCallFrame(MF) || hasFP(MF);
+ }
+
/// hasReservedSpillSlot - Return true if target has reserved a spill slot in
/// the stack frame of the given function for the specified register. e.g. On
/// x86, if the frame register is required, the first fixed stack object is
diff --git a/lib/CodeGen/PrologEpilogInserter.cpp b/lib/CodeGen/PrologEpilogInserter.cpp
index 040259e..138e711 100644
--- a/lib/CodeGen/PrologEpilogInserter.cpp
+++ b/lib/CodeGen/PrologEpilogInserter.cpp
@@ -175,9 +175,10 @@
MachineBasicBlock::iterator I = *i;
// If call frames are not being included as part of the stack frame, and
- // there is no dynamic allocation (therefore referencing frame slots off
- // sp), leave the pseudo ops alone. We'll eliminate them later.
- if (RegInfo->hasReservedCallFrame(Fn) || RegInfo->hasFP(Fn))
+ // the target doesn't indicate otherwise, remove the call frame pseudos
+ // here. The sub/add sp instruction pairs are still inserted, but we don't
+ // need to track the SP adjustment for frame index elimination.
+ if (RegInfo->canSimplifyCallFramePseudos(Fn))
RegInfo->eliminateCallFramePseudoInstr(Fn, *I->getParent(), I);
}
}
diff --git a/lib/Target/ARM/ARMBaseRegisterInfo.cpp b/lib/Target/ARM/ARMBaseRegisterInfo.cpp
index b1dad39..361a90f 100644
--- a/lib/Target/ARM/ARMBaseRegisterInfo.cpp
+++ b/lib/Target/ARM/ARMBaseRegisterInfo.cpp
@@ -1085,6 +1085,16 @@
return !MF.getFrameInfo()->hasVarSizedObjects();
}
+// canSimplifyCallFramePseudos - If there is a reserved call frame, the
+// call frame pseudos can be simplified. Unlike most targets, having a FP
+// is not sufficient here since we still may reference some objects via SP
+// even when FP is available in Thumb2 mode.
+bool ARMBaseRegisterInfo::
+canSimplifyCallFramePseudos(MachineFunction &MF) const {
+ ARMFunctionInfo *AFI = MF.getInfo<ARMFunctionInfo>();
+ return hasReservedCallFrame(MF) || (AFI->isThumb1OnlyFunction() && hasFP(MF));
+}
+
static void
emitSPUpdate(bool isARM,
MachineBasicBlock &MBB, MachineBasicBlock::iterator &MBBI,
diff --git a/lib/Target/ARM/ARMBaseRegisterInfo.h b/lib/Target/ARM/ARMBaseRegisterInfo.h
index 33ba21d..64f6ff1 100644
--- a/lib/Target/ARM/ARMBaseRegisterInfo.h
+++ b/lib/Target/ARM/ARMBaseRegisterInfo.h
@@ -138,6 +138,7 @@
virtual bool requiresFrameIndexScavenging(const MachineFunction &MF) const;
virtual bool hasReservedCallFrame(MachineFunction &MF) const;
+ virtual bool canSimplifyCallFramePseudos(MachineFunction &MF) const;
virtual void eliminateCallFramePseudoInstr(MachineFunction &MF,
MachineBasicBlock &MBB,
diff --git a/lib/Target/ARM/ARMInstrInfo.td b/lib/Target/ARM/ARMInstrInfo.td
index 262695b..ca917fa 100644
--- a/lib/Target/ARM/ARMInstrInfo.td
+++ b/lib/Target/ARM/ARMInstrInfo.td
@@ -635,7 +635,10 @@
i32imm:$size), NoItinerary,
"${instid:label} ${cpidx:cpentry}", []>;
-let Defs = [SP], Uses = [SP] in {
+// FIXME: Marking these as hasSideEffects is necessary to prevent machine DCE
+// from removing one half of the matched pairs. That breaks PEI, which assumes
+// these will always be in pairs, and asserts if it finds otherwise. Better way?
+let Defs = [SP], Uses = [SP], hasSideEffects = 1 in {
def ADJCALLSTACKUP :
PseudoInst<(outs), (ins i32imm:$amt1, i32imm:$amt2, pred:$p), NoItinerary,
"@ ADJCALLSTACKUP $amt1",
diff --git a/lib/Target/ARM/ARMInstrThumb.td b/lib/Target/ARM/ARMInstrThumb.td
index b1f0459..e8d3e22 100644
--- a/lib/Target/ARM/ARMInstrThumb.td
+++ b/lib/Target/ARM/ARMInstrThumb.td
@@ -120,7 +120,10 @@
// Miscellaneous Instructions.
//
-let Defs = [SP], Uses = [SP] in {
+// FIXME: Marking these as hasSideEffects is necessary to prevent machine DCE
+// from removing one half of the matched pairs. That breaks PEI, which assumes
+// these will always be in pairs, and asserts if it finds otherwise. Better way?
+let Defs = [SP], Uses = [SP], hasSideEffects = 1 in {
def tADJCALLSTACKUP :
PseudoInst<(outs), (ins i32imm:$amt1, i32imm:$amt2), NoItinerary,
"@ tADJCALLSTACKUP $amt1",