Skip to content

Commit f2bc7b7

Browse files
citymarinaahmedbougacha
citymarina
and
ahmedbougacha
authored
[AArch64] Allow the clang.arc.attachedcall marker to be optional (#138694)
Now that the clang.arc.attachedcall bundle requires having an operand, which we emit a call to in the RVMARKER sequence, we can achieve our real goal: make the marker NOP optional. The intention is that a new ObjC runtime call will be introduced, which doesn't require the NOP to be present, but must be adjacent to the possibly-autorelease-returning call (that the bundle is attached to). This is achieved by having ISel embed whether the marker is necessary with an additional boolean target immediate operand. Co-authored-by: Ahmed Bougacha <[email protected]>
1 parent 2017831 commit f2bc7b7

File tree

6 files changed

+74
-30
lines changed

6 files changed

+74
-30
lines changed

‎llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -836,21 +836,22 @@ bool AArch64ExpandPseudo::expandCALL_RVMARKER(
836836
MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI) {
837837
// Expand CALL_RVMARKER pseudo to:
838838
// - a branch to the call target, followed by
839-
// - the special `mov x29, x29` marker, and
839+
// - the special `mov x29, x29` marker, if necessary, and
840840
// - another branch, to the runtime function
841841
// Mark the sequence as bundle, to avoid passes moving other code in between.
842842
MachineInstr &MI = *MBBI;
843843
MachineOperand &RVTarget = MI.getOperand(0);
844+
bool DoEmitMarker = MI.getOperand(1).getImm();
844845
assert(RVTarget.isGlobal() && "invalid operand for attached call");
845846

846847
MachineInstr *OriginalCall = nullptr;
847848

848849
if (MI.getOpcode() == AArch64::BLRA_RVMARKER) {
849850
// ptrauth call.
850-
const MachineOperand &CallTarget = MI.getOperand(1);
851-
const MachineOperand &Key = MI.getOperand(2);
852-
const MachineOperand &IntDisc = MI.getOperand(3);
853-
const MachineOperand &AddrDisc = MI.getOperand(4);
851+
const MachineOperand &CallTarget = MI.getOperand(2);
852+
const MachineOperand &Key = MI.getOperand(3);
853+
const MachineOperand &IntDisc = MI.getOperand(4);
854+
const MachineOperand &AddrDisc = MI.getOperand(5);
854855

855856
assert((Key.getImm() == AArch64PACKey::IA ||
856857
Key.getImm() == AArch64PACKey::IB) &&
@@ -859,19 +860,20 @@ bool AArch64ExpandPseudo::expandCALL_RVMARKER(
859860
MachineOperand Ops[] = {CallTarget, Key, IntDisc, AddrDisc};
860861

861862
OriginalCall = createCallWithOps(MBB, MBBI, TII, AArch64::BLRA, Ops,
862-
/*RegMaskStartIdx=*/5);
863+
/*RegMaskStartIdx=*/6);
863864
} else {
864865
assert(MI.getOpcode() == AArch64::BLR_RVMARKER && "unknown rvmarker MI");
865-
OriginalCall = createCall(MBB, MBBI, TII, MI.getOperand(1),
866+
OriginalCall = createCall(MBB, MBBI, TII, MI.getOperand(2),
866867
// Regmask starts after the RV and call targets.
867-
/*RegMaskStartIdx=*/2);
868+
/*RegMaskStartIdx=*/3);
868869
}
869870

870-
BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(AArch64::ORRXrs))
871-
.addReg(AArch64::FP, RegState::Define)
872-
.addReg(AArch64::XZR)
873-
.addReg(AArch64::FP)
874-
.addImm(0);
871+
if (DoEmitMarker)
872+
BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(AArch64::ORRXrs))
873+
.addReg(AArch64::FP, RegState::Define)
874+
.addReg(AArch64::XZR)
875+
.addReg(AArch64::FP)
876+
.addImm(0);
875877

876878
auto *RVCall = BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(AArch64::BL))
877879
.add(RVTarget)

‎llvm/lib/Target/AArch64/AArch64ISelLowering.cpp‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9543,6 +9543,13 @@ AArch64TargetLowering::LowerCall(CallLoweringInfo &CLI,
95439543
Function *ARCFn = *objcarc::getAttachedARCFunction(CLI.CB);
95449544
auto GA = DAG.getTargetGlobalAddress(ARCFn, DL, PtrVT);
95459545
Ops.insert(Ops.begin() + 1, GA);
9546+
9547+
// We may or may not need to emit both the marker and the retain/claim call.
9548+
// Do what the frontend tells us: if the rvmarker module flag is present,
9549+
// emit the marker. Always emit the call regardless.
9550+
// Tell the pseudo expansion using an additional boolean op.
9551+
SDValue DoEmitMarker = DAG.getTargetConstant(true, DL, MVT::i32);
9552+
Ops.insert(Ops.begin() + 2, DoEmitMarker);
95469553
} else if (CallConv == CallingConv::ARM64EC_Thunk_X64) {
95479554
Opc = AArch64ISD::CALL_ARM64EC_TO_X64;
95489555
} else if (GuardWithBTI) {

‎llvm/lib/Target/AArch64/AArch64InstrInfo.td‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -754,10 +754,11 @@ def AArch64authtcret: SDNode<"AArch64ISD::AUTH_TC_RETURN",
754754

755755
def AArch64authcall_rvmarker : SDNode<"AArch64ISD::AUTH_CALL_RVMARKER",
756756
SDTypeProfile<0, -1, [SDTCisPtrTy<0>,
757-
SDTCisPtrTy<1>,
758-
SDTCisVT<2, i32>,
759-
SDTCisVT<3, i64>,
760-
SDTCisVT<4, i64>]>,
757+
SDTCisVT<1, i32>,
758+
SDTCisPtrTy<2>,
759+
SDTCisVT<3, i32>,
760+
SDTCisVT<4, i64>,
761+
SDTCisVT<5, i64>]>,
761762
[SDNPHasChain, SDNPOptInGlue, SDNPOutGlue,
762763
SDNPVariadic]>;
763764

@@ -1896,9 +1897,9 @@ let Predicates = [HasPAuth] in {
18961897
}
18971898

18981899
def BLRA_RVMARKER : Pseudo<
1899-
(outs), (ins i64imm:$rvfunc, GPR64noip:$Rn, i32imm:$Key, i64imm:$Disc,
1900-
GPR64:$AddrDisc),
1901-
[(AArch64authcall_rvmarker tglobaladdr:$rvfunc,
1900+
(outs), (ins i64imm:$rvfunc, i32imm:$withmarker, GPR64noip:$Rn,
1901+
i32imm:$Key, i64imm:$Disc, GPR64:$AddrDisc),
1902+
[(AArch64authcall_rvmarker tglobaladdr:$rvfunc, timm:$withmarker,
19021903
GPR64noip:$Rn, timm:$Key, timm:$Disc,
19031904
GPR64:$AddrDisc)]>, Sched<[]> {
19041905
let isCodeGenOnly = 1;
@@ -3293,8 +3294,9 @@ def : Pat<(AArch64call GPR64noip:$Rn),
32933294
(BLRNoIP GPR64noip:$Rn)>,
32943295
Requires<[SLSBLRMitigation]>;
32953296

3296-
def : Pat<(AArch64call_rvmarker (i64 tglobaladdr:$rvfunc), GPR64:$Rn),
3297-
(BLR_RVMARKER tglobaladdr:$rvfunc, GPR64:$Rn)>,
3297+
def : Pat<(AArch64call_rvmarker (i64 tglobaladdr:$rvfunc),
3298+
(i32 timm:$withmarker), GPR64:$Rn),
3299+
(BLR_RVMARKER tglobaladdr:$rvfunc, timm:$withmarker, GPR64:$Rn)>,
32983300
Requires<[NoSLSBLRMitigation]>;
32993301

33003302
def : Pat<(AArch64call_bti GPR64:$Rn),

‎llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1364,6 +1364,13 @@ bool AArch64CallLowering::lowerCall(MachineIRBuilder &MIRBuilder,
13641364
Function *ARCFn = *objcarc::getAttachedARCFunction(Info.CB);
13651365
MIB.addGlobalAddress(ARCFn);
13661366
++CalleeOpNo;
1367+
1368+
// We may or may not need to emit both the marker and the retain/claim call.
1369+
// Do what the frontend tells us: if the rvmarker module flag is present,
1370+
// emit the marker. Always emit the call regardless.
1371+
// Tell the pseudo expansion using an additional boolean op.
1372+
MIB.addImm(true);
1373+
++CalleeOpNo;
13671374
} else if (Info.CFIType) {
13681375
MIB->setCFIType(MF, Info.CFIType->getZExtValue());
13691376
}

‎llvm/test/CodeGen/AArch64/expand-blr-rvmarker-pseudo.mir‎

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,10 @@
2929
ret void
3030
}
3131

32+
define void @test_no_nop() {
33+
ret void
34+
}
35+
3236
declare ptr @attachedcall()
3337

3438
declare ptr @objc_retainAutoreleasedReturnValue()
@@ -54,7 +58,7 @@ body: |
5458
bb.0:
5559
liveins: $lr, $x0
5660
57-
BLR_RVMARKER @attachedcall, $x0, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def dead $x0
61+
BLR_RVMARKER @attachedcall, 1, $x0, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def dead $x0
5862
RET_ReallyLR implicit killed $w0
5963
...
6064

@@ -74,7 +78,7 @@ body: |
7478
bb.0:
7579
liveins: $lr, $x0
7680
77-
BLR_RVMARKER @attachedcall, @foo, $x0, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def dead $x0
81+
BLR_RVMARKER @attachedcall, 1, @foo, $x0, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def dead $x0
7882
RET_ReallyLR implicit killed $w0
7983
...
8084

@@ -94,7 +98,7 @@ body: |
9498
bb.0:
9599
liveins: $lr, $x0, $x1, $x2
96100
97-
BLR_RVMARKER @attachedcall, @foo, $x0, $x1, $x2, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def $x0
101+
BLR_RVMARKER @attachedcall, 1, @foo, $x0, $x1, $x2, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def $x0
98102
RET_ReallyLR
99103
...
100104

@@ -114,7 +118,7 @@ body: |
114118
bb.0:
115119
liveins: $lr, $w0, $w1
116120
117-
BLR_RVMARKER @attachedcall, @foo, $w0, $w1, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def dead $x0
121+
BLR_RVMARKER @attachedcall, 1, @foo, $w0, $w1, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def dead $x0
118122
RET_ReallyLR implicit killed $w0
119123
...
120124

@@ -135,7 +139,7 @@ body: |
135139
bb.0:
136140
liveins: $lr, $x8, $w0, $w1
137141
138-
BLR_RVMARKER @attachedcall, $x8, $w0, $w1, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def dead $x0
142+
BLR_RVMARKER @attachedcall, 1, $x8, $w0, $w1, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def dead $x0
139143
RET_ReallyLR implicit killed $w0
140144
...
141145

@@ -158,6 +162,28 @@ body: |
158162
bb.0:
159163
liveins: $lr
160164
161-
BLR_RVMARKER @objc_retainAutoreleasedReturnValue, @foo, undef $x0, csr_darwin_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def dead $x0
165+
BLR_RVMARKER @objc_retainAutoreleasedReturnValue, 1, @foo, undef $x0, csr_darwin_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def dead $x0
162166
RET_ReallyLR
163167
...
168+
169+
# CHECK-LABEL: : test_no_nop
170+
# CHECK: bb.0:
171+
# CHECK-NEXT: liveins:
172+
# CHECK-NEXT: {{ $}}
173+
# CHECK-NEXT: BUNDLE implicit-def $lr, implicit-def $w30, implicit-def $w30_hi, implicit-def $sp, implicit-def $wsp, implicit-def $wsp_hi, implicit-def dead $x0, implicit $x0, implicit $sp {
174+
# CHECK-NEXT: BLR $x0, csr_aarch64_aapcs, implicit-def $lr, implicit $sp, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def dead $x0
175+
# CHECK-NEXT: BL @attachedcall, implicit-def $lr, implicit internal $sp
176+
# CHECK-NEXT: }
177+
# CHECK-NEXT: RET undef $lr, implicit killed $w0
178+
#
179+
name: test_no_nop
180+
callSites:
181+
- {bb: 0, offset: 0, fwdArgRegs:
182+
- { arg: 0, reg: '$x0' } }
183+
body: |
184+
bb.0:
185+
liveins: $lr, $x0
186+
187+
BLR_RVMARKER @attachedcall, 0, $x0, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def dead $x0
188+
RET_ReallyLR implicit killed $w0
189+
...

‎llvm/test/CodeGen/AArch64/rvmarker-pseudo-expansion-and-outlining.mir‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ body: |
5252
bb.0:
5353
liveins: $lr
5454
55-
BLR_RVMARKER @attachedcall, @cb1, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def $x0
55+
BLR_RVMARKER @attachedcall, 1, @cb1, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def $x0
5656
$w12 = ORRWri $wzr, 1
5757
$w12 = ORRWri $wzr, 1
5858
$w12 = ORRWri $wzr, 1
@@ -71,7 +71,7 @@ body: |
7171
bb.0:
7272
liveins: $lr, $x19, $x20, $lr
7373
74-
BLR_RVMARKER @attachedcall, @cb2, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def $x0
74+
BLR_RVMARKER @attachedcall, 1, @cb2, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def $x0
7575
$w12 = ORRWri $wzr, 1
7676
$w12 = ORRWri $wzr, 1
7777
$w12 = ORRWri $wzr, 1

0 commit comments

Comments
 (0)