[ARM/Thumb] Mark TAILJMP*/TCRETURN* as using LR. #73492
Conversation
TCRETURN/TAILJMP may use LR in the tail-called function to return to the original caller. At the moment, they are not marked as using LR, which can cause mis-compiles, e.g. if the MachineOutliner introduces new calls to outlined functions using BL (see outlined-fn-may-clobber-lr-in-caller.ll for an example). I might be missing a better way to address this issue, but without marking LR as used, I am not sure how liveness of LR can be correctly tracked by the MachineOutliner to decide whether LR needs to be preserved across outlined calls. The current version is causing some MachineVerifier failures in some cases, due to LR not being marked as live-in properly directly after ISel (see below for verifier error message). I am not really sure why this is happening. | *** Bad machine code: Using an undefined physical register *** | - function: func | - basic block: %bb.2 (0x12280bc10) | - instruction: TCRETURNri %1:tcgpr, 0, <regmask $lr $d8 $d9 $d10 $d11 $d12 $d13 $d14 $d15 $q4 $q5 $q6 $q7 $r4 $r5 $r6 $r7 $r8 $r10 $r11 $s16 $s17 $s18 $s19 $s20 $s21 $s22 $s23 $s24 $s25 $s26 $s27 $s28 and 33 more...>, implicit $sp, implicit $lr, implicit $r0 | - operand 4: implicit $lr | LLVM ERROR: Found 1 machine code errors.
fhahn
requested review from
ahmedbougacha,
TNorthover,
momchil-velikov and
davemgreen
|
@llvm/pr-subscribers-backend-arm Author: Florian Hahn (fhahn) ChangesTCRETURN/TAILJMP may use LR in the tail-called function to return to the original caller. At the moment, they are not marked as using LR, which can cause mis-compiles, e.g. if the MachineOutliner introduces new calls to outlined functions using BL (see outlined-fn-may-clobber-lr-in-caller.ll for an example). I might be missing a better way to address this issue, but without marking LR as used, I am not sure how liveness of LR can be correctly tracked by the MachineOutliner to decide whether LR needs to be preserved across outlined calls. The current version is causing some MachineVerifier failures in some cases, due to LR not being marked as live-in properly directly after ISel (see below for verifier error message). I am not really sure why this is happening. | *** Bad machine code: Using an undefined physical register *** Full diff: https://github.com/llvm/llvm-project/pull/73492.diff 4 Files Affected:
diff --git a/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp b/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
index c09879fd9c2beb3..d3f896435862cca 100644
--- a/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
+++ b/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
@@ -6587,6 +6587,10 @@ void ARMBaseInstrInfo::buildOutlinedFrame(
// For thunk outlining, rewrite the last instruction from a call to a
// tail-call.
if (OF.FrameConstructionID == MachineOutlinerThunk) {
+ // LR is used by the tail call.
+ if (!MBB.isLiveIn(ARM::LR))
+ MBB.addLiveIn(ARM::LR);
+
MachineInstr *Call = &*--MBB.instr_end();
bool isThumb = Subtarget.isThumb();
unsigned FuncOp = isThumb ? 2 : 0;
diff --git a/llvm/lib/Target/ARM/ARMInstrInfo.td b/llvm/lib/Target/ARM/ARMInstrInfo.td
index 812b5730875d5d4..3e0df3afccb1028 100644
--- a/llvm/lib/Target/ARM/ARMInstrInfo.td
+++ b/llvm/lib/Target/ARM/ARMInstrInfo.td
@@ -2668,7 +2668,7 @@ def BXJ : ABI<0b0001, (outs), (ins GPR:$func), NoItinerary, "bxj", "\t$func",
// Tail calls.
-let isCall = 1, isTerminator = 1, isReturn = 1, isBarrier = 1, Uses = [SP] in {
+let isCall = 1, isTerminator = 1, isReturn = 1, isBarrier = 1, Uses = [SP,LR] in {
def TCRETURNdi : PseudoInst<(outs), (ins i32imm:$dst, i32imm:$SPDiff), IIC_Br, []>,
Sched<[WriteBr]>;
diff --git a/llvm/lib/Target/ARM/ARMInstrThumb2.td b/llvm/lib/Target/ARM/ARMInstrThumb2.td
index acd46e8093aa78d..0b1ae71b5bcb48b 100644
--- a/llvm/lib/Target/ARM/ARMInstrThumb2.td
+++ b/llvm/lib/Target/ARM/ARMInstrThumb2.td
@@ -4035,7 +4035,7 @@ def t2Bcc : T2I<(outs), (ins brtarget:$target), IIC_Br,
// Windows SEH unwinding also needs a strict t2 branch for tail calls.
let isCall = 1, isTerminator = 1, isReturn = 1, isBarrier = 1 in {
// IOS version.
- let Uses = [SP] in
+ let Uses = [SP,LR] in
def tTAILJMPd: tPseudoExpand<(outs),
(ins thumb_br_target:$dst, pred:$p),
4, IIC_Br, [],
diff --git a/llvm/test/CodeGen/Thumb2/outlined-fn-may-clobber-lr-in-caller.ll b/llvm/test/CodeGen/Thumb2/outlined-fn-may-clobber-lr-in-caller.ll
index d81d008b44bed89..a9e59cecfc2417b 100644
--- a/llvm/test/CodeGen/Thumb2/outlined-fn-may-clobber-lr-in-caller.ll
+++ b/llvm/test/CodeGen/Thumb2/outlined-fn-may-clobber-lr-in-caller.ll
@@ -16,6 +16,8 @@ target datalayout = "e-m:o-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
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
+; CHECK-NEXT: .save {r7, lr}
+; CHECK-NEXT: push {r7, lr}
; CHECK-NEXT: cmp r1, #2
; CHECK-NEXT: beq .LBB0_3
; CHECK-NEXT: @ %bb.1: @ %bb
@@ -32,10 +34,9 @@ define void @test(ptr nocapture noundef writeonly %arg, i32 noundef %arg1, i8 no
; CHECK-NEXT: add.w r1, r2, r1, lsl #2
; CHECK-NEXT: adds r0, #4
; CHECK-NEXT: movs r2, #30
+; CHECK-NEXT: pop.w {r7, lr}
; CHECK-NEXT: b __aeabi_memcpy
; CHECK-NEXT: .LBB0_5: @ %bb24
-; CHECK-NEXT: .save {r7, lr}
-; CHECK-NEXT: push {r7, lr}
; CHECK-NEXT: bl wombat
; CHECK-NEXT: @APP
; CHECK-NEXT: @NO_APP
|
|
Had another closer look and it seems like liveness is correct after Prolog/Epilog insertion, but gets not updated properly by a transform. Will put up an alternatvie fix |
|
Hey @fhahn do you have somewhere else you are tracking this? I think we've seen this in Rust, with Does this match your understanding of the issue? |
|
@jamesmunns @jamesmunns yeah this looks similar. The latest iteration of a potential fix can be found at #73553 with additional discussion. Will pick this up again soon. Any chance you could check if #73553 fixes your issue? |
TCRETURN/TAILJMP may use LR in the tail-called function to return to the original caller. At the moment, they are not marked as using LR, which can cause mis-compiles, e.g. if the MachineOutliner introduces new calls to outlined functions using BL (see outlined-fn-may-clobber-lr-in-caller.ll for an example).
I might be missing a better way to address this issue, but without marking LR as used, I am not sure how liveness of LR can be correctly tracked by the MachineOutliner to decide whether LR needs to be preserved across outlined calls.
The current version is causing some MachineVerifier failures in some cases, due to LR not being marked as live-in properly directly after ISel (see below for verifier error message). I am not really sure why this is happening.
| *** Bad machine code: Using an undefined physical register ***
| - function: func
| - basic block: %bb.2 (0x12280bc10)
| - instruction: TCRETURNri %1:tcgpr, 0, <regmask $lr $d8 $d9 $d10 $d11 $d12 $d13 $d14 $d15 $q4 $q5 $q6 $q7 $r4 $r5 $r6 $r7 $r8 $r10 $r11 $s16 $s17 $s18 $s19 $s20 $s21 $s22 $s23 $s24 $s25 $s26 $s27 $s28 and 33 more...>, implicit $sp, implicit $lr, implicit $r0
| - operand 4: implicit $lr
| LLVM ERROR: Found 1 machine code errors.