Skip to content

Commit

Permalink
[ARM] Check all terms in emitPopInst when clearing Restored for LR. (l…
Browse files Browse the repository at this point in the history
…lvm#75527)

emitPopInst checks a single function exit MBB. If other paths also exit
the function and any of there terminators uses LR implicitly, it is not
save to clear the Restored bit.

Check all terminators for the function before clearing Restored.

This fixes a mis-compile in outlined-fn-may-clobber-lr-in-caller.ll
where the machine-outliner previously introduced BLs that clobbered LR
which in turn is used by the tail call return.

Alternative to llvm#73553

(cherry picked from commit b1a5ee1)
  • Loading branch information
fhahn authored and MingcongBai committed Mar 26, 2024
1 parent 58fbea2 commit f03a987
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 7 deletions.
30 changes: 27 additions & 3 deletions llvm/lib/Target/ARM/ARMFrameLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1645,9 +1645,6 @@ void ARMFrameLowering::emitPopInst(MachineBasicBlock &MBB,
// Fold the return instruction into the LDM.
DeleteRet = true;
LdmOpc = AFI->isThumbFunction() ? ARM::t2LDMIA_RET : ARM::LDMIA_RET;
// We 'restore' LR into PC so it is not live out of the return block:
// Clear Restored bit.
Info.setRestored(false);
}

// If NoGap is true, pop consecutive registers and then leave the rest
Expand Down Expand Up @@ -2769,6 +2766,33 @@ void ARMFrameLowering::determineCalleeSaves(MachineFunction &MF,
AFI->setLRIsSpilled(SavedRegs.test(ARM::LR));
}

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

MachineFrameInfo &MFI = MF.getFrameInfo();
if (!MFI.isCalleeSavedInfoValid())
return;

// Check if all terminators do not implicitly use LR. Then we can 'restore' LR
// into PC so it is not live out of the return block: Clear the Restored bit
// in that case.
for (CalleeSavedInfo &Info : MFI.getCalleeSavedInfo()) {
if (Info.getReg() != ARM::LR)
continue;
if (all_of(MF, [](const MachineBasicBlock &MBB) {
return all_of(MBB.terminators(), [](const MachineInstr &Term) {
return !Term.isReturn() || Term.getOpcode() == ARM::LDMIA_RET ||
Term.getOpcode() == ARM::t2LDMIA_RET ||
Term.getOpcode() == ARM::tPOP_RET;
});
})) {
Info.setRestored(false);
break;
}
}
}

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

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

void adjustForSegmentedStacks(MachineFunction &MF,
MachineBasicBlock &MBB) const override;

Expand Down
14 changes: 10 additions & 4 deletions llvm/test/CodeGen/Thumb2/outlined-fn-may-clobber-lr-in-caller.ll
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ target datalayout = "e-m:o-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"

; Test case to make sure calling an outlined function does not clobber LR used
; by a tail call in caller.
; FIXME: Currently bl OUTLINED_FUNCTION_0 clobbers LR, which in turn is used
; by the later call to memcpy to return to the caller.
define void @test(ptr nocapture noundef writeonly %arg, i32 noundef %arg1, i8 noundef zeroext %arg2) unnamed_addr #0 {
; CHECK-LABEL: test:
; CHECK: @ %bb.0: @ %bb
Expand All @@ -22,11 +20,19 @@ define void @test(ptr nocapture noundef writeonly %arg, i32 noundef %arg1, i8 no
; CHECK-NEXT: cmp r1, #1
; CHECK-NEXT: bne .LBB0_5
; CHECK-NEXT: @ %bb.2: @ %bb4
; CHECK-NEXT: bl OUTLINED_FUNCTION_0
; CHECK-NEXT: movs r1, #1
; CHECK-NEXT: strb.w r1, [r0, #36]
; CHECK-NEXT: movs r1, #30
; CHECK-NEXT: strb.w r1, [r0, #34]
; CHECK-NEXT: add.w r1, r2, r2, lsl #3
; CHECK-NEXT: ldr r2, .LCPI0_1
; CHECK-NEXT: b .LBB0_4
; CHECK-NEXT: .LBB0_3: @ %bb14
; CHECK-NEXT: bl OUTLINED_FUNCTION_0
; CHECK-NEXT: movs r1, #1
; CHECK-NEXT: strb.w r1, [r0, #36]
; CHECK-NEXT: movs r1, #30
; CHECK-NEXT: strb.w r1, [r0, #34]
; CHECK-NEXT: add.w r1, r2, r2, lsl #3
; CHECK-NEXT: ldr r2, .LCPI0_0
; CHECK-NEXT: .LBB0_4: @ %bb4
; CHECK-NEXT: add.w r1, r2, r1, lsl #2
Expand Down

0 comments on commit f03a987

Please sign in to comment.