[llvm] r322231 - Revert "AArch64: Fix emergency spillslot being out of reach for large callframes"

Matthias Braun via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 10 14:36:28 PST 2018


Author: matze
Date: Wed Jan 10 14:36:28 2018
New Revision: 322231

URL: http://llvm.org/viewvc/llvm-project?rev=322231&view=rev
Log:
Revert "AArch64: Fix emergency spillslot being out of reach for large callframes"

Revert for now as the testcase is hitting a pre-existing verifier error
that manifest as a failure when expensive checks are enabled (or
-verify-machineinstrs) is used.

This reverts commit r322200.

Removed:
    llvm/trunk/test/CodeGen/AArch64/big-callframe.ll
Modified:
    llvm/trunk/include/llvm/CodeGen/TargetSubtargetInfo.h
    llvm/trunk/lib/CodeGen/MIRParser/MIRParser.cpp
    llvm/trunk/lib/CodeGen/TargetSubtargetInfo.cpp
    llvm/trunk/lib/Target/AArch64/AArch64FrameLowering.cpp
    llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp
    llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.h
    llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.cpp
    llvm/trunk/lib/Target/AArch64/AArch64Subtarget.cpp
    llvm/trunk/lib/Target/AArch64/AArch64Subtarget.h

Modified: llvm/trunk/include/llvm/CodeGen/TargetSubtargetInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/TargetSubtargetInfo.h?rev=322231&r1=322230&r2=322231&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/TargetSubtargetInfo.h (original)
+++ llvm/trunk/include/llvm/CodeGen/TargetSubtargetInfo.h Wed Jan 10 14:36:28 2018
@@ -248,9 +248,6 @@ public:
   /// Returns string representation of scheduler comment
   std::string getSchedInfoStr(const MachineInstr &MI) const override;
   std::string getSchedInfoStr(MCInst const &MCI) const override;
-
-  /// This is called after a .mir file was loaded.
-  virtual void mirFileLoaded(MachineFunction &MF) const;
 };
 
 } // end namespace llvm

Modified: llvm/trunk/lib/CodeGen/MIRParser/MIRParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MIRParser/MIRParser.cpp?rev=322231&r1=322230&r2=322231&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/MIRParser/MIRParser.cpp (original)
+++ llvm/trunk/lib/CodeGen/MIRParser/MIRParser.cpp Wed Jan 10 14:36:28 2018
@@ -417,8 +417,6 @@ MIRParserImpl::initializeMachineFunction
 
   computeFunctionProperties(MF);
 
-  MF.getSubtarget().mirFileLoaded(MF);
-
   MF.verify();
   return false;
 }

Modified: llvm/trunk/lib/CodeGen/TargetSubtargetInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/TargetSubtargetInfo.cpp?rev=322231&r1=322230&r2=322231&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/TargetSubtargetInfo.cpp (original)
+++ llvm/trunk/lib/CodeGen/TargetSubtargetInfo.cpp Wed Jan 10 14:36:28 2018
@@ -111,6 +111,3 @@ std::string TargetSubtargetInfo::getSche
       TSchedModel.computeInstrRThroughput(MCI.getOpcode());
   return createSchedInfoStr(Latency, RThroughput);
 }
-
-void TargetSubtargetInfo::mirFileLoaded(MachineFunction &MF) const {
-}

Modified: llvm/trunk/lib/Target/AArch64/AArch64FrameLowering.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64FrameLowering.cpp?rev=322231&r1=322230&r2=322231&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AArch64/AArch64FrameLowering.cpp (original)
+++ llvm/trunk/lib/Target/AArch64/AArch64FrameLowering.cpp Wed Jan 10 14:36:28 2018
@@ -142,12 +142,6 @@ static cl::opt<bool> EnableRedZone("aarc
 
 STATISTIC(NumRedZoneFunctions, "Number of functions using red zone");
 
-/// This is the biggest offset to the stack pointer we can encode in aarch64
-/// instructions (without using a separate calculation and a temp register).
-/// Note that the exception here are vector stores/loads which cannot encode any
-/// displacements (see estimateRSStackSizeLimit(), isAArch64FrameOffsetLegal()).
-static const unsigned DefaultSafeSPDisplacement = 255;
-
 /// Look at each instruction that references stack frames and return the stack
 /// size limit beyond which some of these instructions will require a scratch
 /// register during their expansion later.
@@ -173,7 +167,7 @@ static unsigned estimateRSStackSizeLimit
       }
     }
   }
-  return DefaultSafeSPDisplacement;
+  return 255;
 }
 
 bool AArch64FrameLowering::canUseRedZone(const MachineFunction &MF) const {
@@ -197,25 +191,11 @@ bool AArch64FrameLowering::hasFP(const M
   const MachineFrameInfo &MFI = MF.getFrameInfo();
   const TargetRegisterInfo *RegInfo = MF.getSubtarget().getRegisterInfo();
   // Retain behavior of always omitting the FP for leaf functions when possible.
-  if (MFI.hasCalls() && MF.getTarget().Options.DisableFramePointerElim(MF))
-    return true;
-  if (MFI.hasVarSizedObjects() || MFI.isFrameAddressTaken() ||
-      MFI.hasStackMap() || MFI.hasPatchPoint() ||
-      RegInfo->needsStackRealignment(MF))
-    return true;
-  // With large callframes around we may need to use FP to access the scavenging
-  // emergency spillslot.
-  //
-  // Unfortunately some calls to hasFP() like machine verifier ->
-  // getReservedReg() -> hasFP in the middle of global isel are too early
-  // to know the max call frame size. Hopefully conservatively returning "true"
-  // in those cases is fine.
-  // DefaultSafeSPDisplacement is fine as we only emergency spill GP regs.
-  if (!MFI.isMaxCallFrameSizeComputed() ||
-      MFI.getMaxCallFrameSize() > DefaultSafeSPDisplacement)
-    return true;
-
-  return false;
+  return (MFI.hasCalls() &&
+          MF.getTarget().Options.DisableFramePointerElim(MF)) ||
+         MFI.hasVarSizedObjects() || MFI.isFrameAddressTaken() ||
+         MFI.hasStackMap() || MFI.hasPatchPoint() ||
+         RegInfo->needsStackRealignment(MF);
 }
 
 /// hasReservedCallFrame - Under normal circumstances, when a frame pointer is

Modified: llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp?rev=322231&r1=322230&r2=322231&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp (original)
+++ llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp Wed Jan 10 14:36:28 2018
@@ -10974,8 +10974,3 @@ AArch64TargetLowering::getVaListSizeInBi
 
   return 3 * getPointerTy(DL).getSizeInBits() + 2 * 32;
 }
-
-void AArch64TargetLowering::finalizeLowering(MachineFunction &MF) const {
-  MF.getFrameInfo().computeMaxCallFrameSize(MF);
-  TargetLoweringBase::finalizeLowering(MF);
-}

Modified: llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.h?rev=322231&r1=322230&r2=322231&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.h (original)
+++ llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.h Wed Jan 10 14:36:28 2018
@@ -647,8 +647,6 @@ private:
                           SelectionDAG &DAG) const override;
 
   bool shouldNormalizeToSelectSequence(LLVMContext &, EVT) const override;
-
-  void finalizeLowering(MachineFunction &MF) const override;
 };
 
 namespace AArch64 {

Modified: llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.cpp?rev=322231&r1=322230&r2=322231&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.cpp (original)
+++ llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.cpp Wed Jan 10 14:36:28 2018
@@ -225,13 +225,11 @@ bool AArch64RegisterInfo::requiresVirtua
 
 bool
 AArch64RegisterInfo::useFPForScavengingIndex(const MachineFunction &MF) const {
-  // This function indicates whether the emergency spillslot should be placed
-  // close to the beginning of the stackframe (closer to FP) or the end
-  // (closer to SP).
-  //
-  // The beginning works most reliably if we have a frame pointer.
-  const AArch64FrameLowering &TFI = *getFrameLowering(MF);
-  return TFI.hasFP(MF);
+  const MachineFrameInfo &MFI = MF.getFrameInfo();
+  // AArch64FrameLowering::resolveFrameIndexReference() can always fall back
+  // to the stack pointer, so only put the emergency spill slot next to the
+  // FP when there's no better way to access it (SP or base pointer).
+  return MFI.hasVarSizedObjects() && !hasBasePointer(MF);
 }
 
 bool AArch64RegisterInfo::requiresFrameIndexScavenging(

Modified: llvm/trunk/lib/Target/AArch64/AArch64Subtarget.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64Subtarget.cpp?rev=322231&r1=322230&r2=322231&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AArch64/AArch64Subtarget.cpp (original)
+++ llvm/trunk/lib/Target/AArch64/AArch64Subtarget.cpp Wed Jan 10 14:36:28 2018
@@ -250,13 +250,3 @@ std::unique_ptr<PBQPRAConstraint>
 AArch64Subtarget::getCustomPBQPConstraints() const {
   return balanceFPOps() ? llvm::make_unique<A57ChainingConstraint>() : nullptr;
 }
-
-void AArch64Subtarget::mirFileLoaded(MachineFunction &MF) const {
-  // We usually compute max call frame size after ISel. Do the computation now
-  // if the .mir file didn't specify it. Note that this will probably give you
-  // bogus values after PEI has eliminated the callframe setup/destroy pseudo
-  // instructions, specify explicitely if you need it to be correct.
-  MachineFrameInfo &MFI = MF.getFrameInfo();
-  if (!MFI.isMaxCallFrameSizeComputed())
-    MFI.computeMaxCallFrameSize(MF);
-}

Modified: llvm/trunk/lib/Target/AArch64/AArch64Subtarget.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64Subtarget.h?rev=322231&r1=322230&r2=322231&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AArch64/AArch64Subtarget.h (original)
+++ llvm/trunk/lib/Target/AArch64/AArch64Subtarget.h Wed Jan 10 14:36:28 2018
@@ -326,8 +326,6 @@ public:
       return false;
     }
   }
-
-  void mirFileLoaded(MachineFunction &MF) const override;
 };
 } // End llvm namespace
 

Removed: llvm/trunk/test/CodeGen/AArch64/big-callframe.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/big-callframe.ll?rev=322230&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/AArch64/big-callframe.ll (original)
+++ llvm/trunk/test/CodeGen/AArch64/big-callframe.ll (removed)
@@ -1,15 +0,0 @@
-; RUN: llc -o - %s | FileCheck %s
-; Make sure we use a frame pointer and fp relative addressing for the emergency
-; spillslot when we have gigantic callframes.
-; CHECK-LABEL: func:
-; CHECK: stur {{.*}}, [x29, #{{.*}}] // 8-byte Folded Spill
-; CHECK: ldur {{.*}}, [x29, #{{.*}}] // 8-byte Folded Reload
-target triple = "aarch64--"
-declare void @extfunc([4096 x i64]* byval %p)
-define void @func([4096 x i64]* %z) {
-  %lvar = alloca [31 x i8]
-  %v = load volatile [31 x i8], [31 x i8]* %lvar
-  store volatile [31 x i8] %v, [31 x i8]* %lvar
-  call void @extfunc([4096 x i64]* byval %z)
-  ret void
-}




More information about the llvm-commits mailing list