Skip to content

Commit

Permalink
[ARM] Update IsRestored for LR based on all returns
Browse files Browse the repository at this point in the history
PR llvm#75527 fixed ARMFrameLowering to set the IsRestored flag for LR based
on all of the return instructions in the function, not just one.
However, there is also code in ARMLoadStoreOptimizer which changes
return instructions, but it set IsRestored based on the one instruction
it changed, not the whole function.

The fix is to factor out the code added in llvm#75527, and also call it from
ARMLoadStoreOptimizer if it made a change to return instructions.

Fixes llvm#80287.
  • Loading branch information
ostannard committed Feb 23, 2024
1 parent 3a73ee4 commit b1d72c2
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 22 deletions.
11 changes: 7 additions & 4 deletions llvm/lib/Target/ARM/ARMFrameLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2781,10 +2781,7 @@ void ARMFrameLowering::determineCalleeSaves(MachineFunction &MF,
AFI->setLRIsSpilled(SavedRegs.test(ARM::LR));
}

void ARMFrameLowering::processFunctionBeforeFrameFinalized(
MachineFunction &MF, RegScavenger *RS) const {
TargetFrameLowering::processFunctionBeforeFrameFinalized(MF, RS);

void ARMFrameLowering::updateLRRestored(MachineFunction &MF) const {
MachineFrameInfo &MFI = MF.getFrameInfo();
if (!MFI.isCalleeSavedInfoValid())
return;
Expand All @@ -2808,6 +2805,12 @@ void ARMFrameLowering::processFunctionBeforeFrameFinalized(
}
}

void ARMFrameLowering::processFunctionBeforeFrameFinalized(
MachineFunction &MF, RegScavenger *RS) const {
TargetFrameLowering::processFunctionBeforeFrameFinalized(MF, RS);
updateLRRestored(MF);
}

void ARMFrameLowering::getCalleeSaves(const MachineFunction &MF,
BitVector &SavedRegs) const {
TargetFrameLowering::getCalleeSaves(MF, SavedRegs);
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/Target/ARM/ARMFrameLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ class ARMFrameLowering : public TargetFrameLowering {
void determineCalleeSaves(MachineFunction &MF, BitVector &SavedRegs,
RegScavenger *RS) const override;

void updateLRRestored(MachineFunction &MF) const;
void processFunctionBeforeFrameFinalized(
MachineFunction &MF, RegScavenger *RS = nullptr) const override;

Expand Down
23 changes: 10 additions & 13 deletions llvm/lib/Target/ARM/ARMLoadStoreOptimizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2062,17 +2062,6 @@ bool ARMLoadStoreOpt::MergeReturnIntoLDM(MachineBasicBlock &MBB) {
MO.setReg(ARM::PC);
PrevMI.copyImplicitOps(*MBB.getParent(), *MBBI);
MBB.erase(MBBI);
// We now restore LR into PC so it is not live-out of the return block
// anymore: Clear the CSI Restored bit.
MachineFrameInfo &MFI = MBB.getParent()->getFrameInfo();
// CSI should be fixed after PrologEpilog Insertion
assert(MFI.isCalleeSavedInfoValid() && "CSI should be valid");
for (CalleeSavedInfo &Info : MFI.getCalleeSavedInfo()) {
if (Info.getReg() == ARM::LR) {
Info.setRestored(false);
break;
}
}
return true;
}
}
Expand Down Expand Up @@ -2120,14 +2109,22 @@ bool ARMLoadStoreOpt::runOnMachineFunction(MachineFunction &Fn) {
isThumb2 = AFI->isThumb2Function();
isThumb1 = AFI->isThumbFunction() && !isThumb2;

bool Modified = false;
bool Modified = false, ModifiedLDMReturn = false;
for (MachineBasicBlock &MBB : Fn) {
Modified |= LoadStoreMultipleOpti(MBB);
if (STI->hasV5TOps() && !AFI->shouldSignReturnAddress())
Modified |= MergeReturnIntoLDM(MBB);
ModifiedLDMReturn |= MergeReturnIntoLDM(MBB);
if (isThumb1)
Modified |= CombineMovBx(MBB);
}
Modified |= ModifiedLDMReturn;

// If we merged a BX instruction into an LDM, we need to re-calculate whether
// LR is restored. This check needs to consider the whole function, not just
// the instruction(s) we changed, because there may be other BX returns which
// still need LR to be restored.
if (ModifiedLDMReturn)
Fn.getSubtarget<ARMSubtarget>().getFrameLowering()->updateLRRestored(Fn);

Allocator.DestroyAll();
return Modified;
Expand Down
11 changes: 6 additions & 5 deletions llvm/test/CodeGen/ARM/ldst-opt-lr-restored.ll
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,17 @@ define i32 @foo(ptr %ctx) {
; CHECK: @ %bb.0: @ %entry
; CHECK-NEXT: cbz r0, .LBB0_2
; CHECK-NEXT: @ %bb.1: @ %if.end
; CHECK-NEXT: movw r12, :lower16:val2
; CHECK-NEXT: movw r3, :lower16:val1
; CHECK-NEXT: movw r2, :lower16:val0
; CHECK-NEXT: mov r1, r0
; CHECK-NEXT: movs r0, #0
; CHECK-NEXT: movw r12, :lower16:val2
; CHECK-NEXT: movw r3, :lower16:val1
; CHECK-NEXT: movt r2, :upper16:val0
; CHECK-NEXT: add.w lr, r1, #4
; CHECK-NEXT: movt r12, :upper16:val2
; CHECK-NEXT: movt r3, :upper16:val1
; CHECK-NEXT: stm.w lr, {r2, r3, r12}
; CHECK-NEXT: movt r2, :upper16:val0
; CHECK-NEXT: str r2, [r1, #4]
; CHECK-NEXT: str r3, [r1, #8]
; CHECK-NEXT: str.w r12, [r1, #12]
; CHECK-NEXT: str r0, [r1, #16]
; CHECK-NEXT: bx lr
; CHECK-NEXT: .LBB0_2: @ %if.then
Expand Down

0 comments on commit b1d72c2

Please sign in to comment.