Skip to content

Commit

Permalink
[ARM] Make sure to save/restore LR when we use tBfar.
Browse files Browse the repository at this point in the history
This change does two things. One, it ensures compilation will abort
instead of miscompiling if ARMFrameLowering::determineCalleeSaves
chooses not to save LR in a case where it's necessary.  Two, it changes
the way we estimate the size of a function to be more conservative in
the presence of constant pool entries and jump tables.

EstimateFunctionSizeInBytes probably still isn't really conservative
enough, but I'm not sure how we can come up with a reliable estimate
before constant islands runs.

Differential Revision: https://reviews.llvm.org/D59439



git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@356527 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
efriedma-quic committed Mar 19, 2019
1 parent 6c2d34c commit aae431a
Show file tree
Hide file tree
Showing 4 changed files with 731 additions and 3 deletions.
3 changes: 3 additions & 0 deletions lib/Target/ARM/ARMConstantIslandPass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1647,6 +1647,9 @@ ARMConstantIslands::fixupUnconditionalBr(ImmBranch &Br) {
if (!isThumb1)
llvm_unreachable("fixupUnconditionalBr is Thumb1 only!");

if (!AFI->isLRSpilled())
report_fatal_error("underestimated function size");

// Use BL to implement far jump.
Br.MaxDisp = (1 << 21) * 2;
MI->setDesc(TII->get(ARM::tBfar));
Expand Down
12 changes: 9 additions & 3 deletions lib/Target/ARM/ARMFrameLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "llvm/CodeGen/MachineFunction.h"
#include "llvm/CodeGen/MachineInstr.h"
#include "llvm/CodeGen/MachineInstrBuilder.h"
#include "llvm/CodeGen/MachineJumpTableInfo.h"
#include "llvm/CodeGen/MachineModuleInfo.h"
#include "llvm/CodeGen/MachineOperand.h"
#include "llvm/CodeGen/MachineRegisterInfo.h"
Expand Down Expand Up @@ -1475,13 +1476,17 @@ bool ARMFrameLowering::restoreCalleeSavedRegisters(MachineBasicBlock &MBB,
}

// FIXME: Make generic?
static unsigned GetFunctionSizeInBytes(const MachineFunction &MF,
const ARMBaseInstrInfo &TII) {
static unsigned EstimateFunctionSizeInBytes(const MachineFunction &MF,
const ARMBaseInstrInfo &TII) {
unsigned FnSize = 0;
for (auto &MBB : MF) {
for (auto &MI : MBB)
FnSize += TII.getInstSizeInBytes(MI);
}
if (MF.getJumpTableInfo())
for (auto &Table: MF.getJumpTableInfo()->getJumpTables())
FnSize += Table.MBBs.size() * 4;
FnSize += MF.getConstantPool()->getConstants().size() * 4;
return FnSize;
}

Expand Down Expand Up @@ -1725,7 +1730,7 @@ void ARMFrameLowering::determineCalleeSaves(MachineFunction &MF,

bool ForceLRSpill = false;
if (!LRSpilled && AFI->isThumb1OnlyFunction()) {
unsigned FnSize = GetFunctionSizeInBytes(MF, TII);
unsigned FnSize = EstimateFunctionSizeInBytes(MF, TII);
// Force LR to be spilled if the Thumb function size is > 2048. This enables
// use of BL to implement far jump. If it turns out that it's not needed
// then the branch fix up path will undo it.
Expand Down Expand Up @@ -2027,6 +2032,7 @@ void ARMFrameLowering::determineCalleeSaves(MachineFunction &MF,
SavedRegs.set(ARM::LR);
AFI->setLRIsSpilledForFarJump(true);
}
AFI->setLRIsSpilled(SavedRegs.test(ARM::LR));
}

MachineBasicBlock::iterator ARMFrameLowering::eliminateCallFramePseudoInstr(
Expand Down
7 changes: 7 additions & 0 deletions lib/Target/ARM/ARMMachineFunctionInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ class ARMFunctionInfo : public MachineFunctionInfo {
/// enable far jump.
bool LRSpilledForFarJump = false;

/// LRSpilled - True if the LR register has been for spilled for
/// any reason, so it's legal to emit an ARM::tBfar (i.e. "bl").
bool LRSpilled = false;

/// FramePtrSpillOffset - If HasStackFrame, this records the frame pointer
/// spill stack offset.
unsigned FramePtrSpillOffset = 0;
Expand Down Expand Up @@ -150,6 +154,9 @@ class ARMFunctionInfo : public MachineFunctionInfo {
bool shouldRestoreSPFromFP() const { return RestoreSPFromFP; }
void setShouldRestoreSPFromFP(bool s) { RestoreSPFromFP = s; }

bool isLRSpilled() const { return LRSpilled; }
void setLRIsSpilled(bool s) { LRSpilled = s; }

bool isLRSpilledForFarJump() const { return LRSpilledForFarJump; }
void setLRIsSpilledForFarJump(bool s) { LRSpilledForFarJump = s; }

Expand Down
Loading

0 comments on commit aae431a

Please sign in to comment.