Updated version of r96634 (which was reverted due to failing 176.gcc and
authorJim Grosbach <grosbach@apple.com>
Mon, 22 Feb 2010 23:10:38 +0000 (23:10 +0000)
committerJim Grosbach <grosbach@apple.com>
Mon, 22 Feb 2010 23:10:38 +0000 (23:10 +0000)
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

include/llvm/Target/TargetRegisterInfo.h
lib/CodeGen/PrologEpilogInserter.cpp
lib/Target/ARM/ARMBaseRegisterInfo.cpp
lib/Target/ARM/ARMBaseRegisterInfo.h
lib/Target/ARM/ARMInstrInfo.td
lib/Target/ARM/ARMInstrThumb.td

index 65b60f7a9366aa92847a6ff837ed62e17e420c9b..212cc93401ae116655a18507c9a7fed0ee2810dc 100644 (file)
@@ -587,6 +587,17 @@ public:
     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
index 040259e15c529e73c4775e5416f5d50e47931732..138e7110306b3800eaa4ce9cf7594c5ac52b00e9 100644 (file)
@@ -175,9 +175,10 @@ void PEI::calculateCallsInformation(MachineFunction &Fn) {
     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);
   }
 }
index b1dad39939108e488f2ffef343626214e384e179..361a90fd48d02152847d02bb6ce7c263ae5bd96b 100644 (file)
@@ -1085,6 +1085,16 @@ hasReservedCallFrame(MachineFunction &MF) const {
   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,
index 33ba21dcb8f07fed93819f56661e3d5350b62485..64f6ff1cb3d14bbce55118ea325bf8bc92850872 100644 (file)
@@ -138,6 +138,7 @@ public:
   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,
index 262695b03c29455c5c1ff396a1cac04839934af2..ca917fa120c6704e4776921d3c49d4eb0d43616c 100644 (file)
@@ -635,7 +635,10 @@ PseudoInst<(outs), (ins cpinst_operand:$instid, cpinst_operand:$cpidx,
                     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",
index b1f0459501d1adb804d871bef221fb5965af8daa..e8d3e228ec313c72e19af412c6ac2c0c9930e454 100644 (file)
@@ -120,7 +120,10 @@ def t_addrmode_sp : Operand<i32>,
 //  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",