Skip to content

Commit bac0e78

Browse files
authored
aarch64: Disable csdb emission by default (#12932)
* aarch64: Disable csdb emission by default This has a massive performance penalty on macOS, for example, and peer compilers are not emitting this as part of on-by-default mitigations. This commit preserves the option to emit it with an aarch64-specific `use_csdb` flag, but the default is now `false` meaning that this is not emitted by default. Closes #12789 * Fix tests * Fix tests & review comments * Use ISLE rule introduced
1 parent f3156fe commit bac0e78

61 files changed

Lines changed: 1668 additions & 221 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

cranelift/codegen/meta/src/isa/arm64.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,12 @@ pub(crate) fn define() -> TargetIsa {
5454
"",
5555
false,
5656
);
57+
settings.add_bool(
58+
"use_csdb",
59+
"Use the `csdb` instruction when spectre mitigations are enabled",
60+
"",
61+
false,
62+
);
5763

5864
TargetIsa::new("arm64", settings.build())
5965
}

cranelift/codegen/src/isa/aarch64/inst.isle

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1822,6 +1822,9 @@
18221822
(decl pure use_fp16 () bool)
18231823
(extern constructor use_fp16 use_fp16)
18241824

1825+
(decl pure use_csdb () bool)
1826+
(extern constructor use_csdb use_csdb)
1827+
18251828
;; Extractor helpers for various immediate constants ;;;;;;;;;;;;;;;;;;;;;;;;;;
18261829

18271830
(decl pure partial move_wide_const_from_u64 (Type u64) MoveWideConst)

cranelift/codegen/src/isa/aarch64/inst/emit.rs

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
use cranelift_control::ControlPlane;
44

55
use crate::ir::{self, types::*};
6+
use crate::isa::aarch64;
67
use crate::isa::aarch64::inst::*;
78
use crate::trace;
89

@@ -711,12 +712,15 @@ impl EmitState {
711712
}
712713

713714
/// Constant state used during function compilation.
714-
pub struct EmitInfo(settings::Flags);
715+
pub struct EmitInfo {
716+
flags: settings::Flags,
717+
isa_flags: aarch64::settings::Flags,
718+
}
715719

716720
impl EmitInfo {
717721
/// Create a constant state for emission of instructions.
718-
pub fn new(flags: settings::Flags) -> Self {
719-
Self(flags)
722+
pub fn new(flags: settings::Flags, isa_flags: aarch64::settings::Flags) -> Self {
723+
Self { flags, isa_flags }
720724
}
721725
}
722726

@@ -3188,7 +3192,9 @@ impl MachInstEmit for Inst {
31883192
inst.emit(sink, emit_info, state);
31893193
// Prevent any data value speculation if spectre mitigations are
31903194
// enabled.
3191-
if emit_info.0.enable_table_access_spectre_mitigation() {
3195+
if emit_info.flags.enable_table_access_spectre_mitigation()
3196+
&& emit_info.isa_flags.use_csdb()
3197+
{
31923198
Inst::Csdb.emit(sink, emit_info, state);
31933199
}
31943200

@@ -3560,7 +3566,7 @@ impl MachInstEmit for Inst {
35603566
}
35613567

35623568
&Inst::StackProbeLoop { start, end, step } => {
3563-
assert!(emit_info.0.enable_probestack());
3569+
assert!(emit_info.flags.enable_probestack());
35643570

35653571
// The loop generated here uses `start` as a counter register to
35663572
// count backwards until negating it exceeds `end`. In other
@@ -3647,9 +3653,11 @@ fn emit_return_call_common_sequence<T>(
36473653
state: &mut EmitState,
36483654
info: &ReturnCallInfo<T>,
36493655
) {
3650-
for inst in
3651-
AArch64MachineDeps::gen_clobber_restore(CallConv::Tail, &emit_info.0, state.frame_layout())
3652-
{
3656+
for inst in AArch64MachineDeps::gen_clobber_restore(
3657+
CallConv::Tail,
3658+
&emit_info.flags,
3659+
state.frame_layout(),
3660+
) {
36533661
inst.emit(sink, emit_info, state);
36543662
}
36553663

cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use crate::ir::types::*;
22
use crate::ir::{ExternalName, TrapCode};
3+
use crate::isa::aarch64;
34
use crate::isa::aarch64::inst::*;
45

56
use alloc::boxed::Box;
@@ -7929,7 +7930,8 @@ fn test_aarch64_binemit() {
79297930
insns.push((Inst::Fence {}, "BF3B03D5", "dmb ish"));
79307931

79317932
let flags = settings::Flags::new(settings::builder());
7932-
let emit_info = EmitInfo::new(flags);
7933+
let isa_flags = aarch64::settings::Flags::new(&flags, &aarch64::settings::builder());
7934+
let emit_info = EmitInfo::new(flags, isa_flags);
79337935
for (insn, expected_encoding, expected_printing) in insns {
79347936
println!("AArch64: {insn:?}, {expected_encoding}, {expected_printing}");
79357937

cranelift/codegen/src/isa/aarch64/lower.isle

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2266,6 +2266,14 @@
22662266

22672267
;;;; Rules for `select_spectre_guard` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
22682268

2269+
(decl maybe_csdb_after_select (ValueRegs) ValueRegs)
2270+
(rule (maybe_csdb_after_select dst)
2271+
(if-let true (use_csdb))
2272+
(let ((_ InstOutput (side_effect (csdb)))) dst))
2273+
(rule (maybe_csdb_after_select dst)
2274+
(if-let false (use_csdb))
2275+
dst)
2276+
22692277
(rule (lower (has_type ty
22702278
(select_spectre_guard _ (maybe_uextend (icmp _ cc x @ (value_type in_ty) y))
22712279
if_true
@@ -2276,9 +2284,8 @@
22762284
(cond_code (flags_and_cc_cc comparison))
22772285
ty
22782286
if_true
2279-
if_false))
2280-
(_ InstOutput (side_effect (csdb))))
2281-
dst))
2287+
if_false)))
2288+
(maybe_csdb_after_select dst)))
22822289

22832290
(rule -1 (lower (has_type ty (select_spectre_guard _ rcond @ (value_type (fits_in_64 _)) rn rm)))
22842291
(let ((rcond Reg (put_in_reg_zext64 rcond)))

cranelift/codegen/src/isa/aarch64/lower/isle.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,10 @@ impl Context for IsleContext<'_, '_, MInst, AArch64Backend> {
181181
self.backend.isa_flags.has_fp16()
182182
}
183183

184+
fn use_csdb(&mut self) -> bool {
185+
self.backend.isa_flags.use_csdb()
186+
}
187+
184188
fn move_wide_const_from_u64(&mut self, ty: Type, n: u64) -> Option<MoveWideConst> {
185189
let bits = ty.bits();
186190
let n = if bits < 64 {

cranelift/codegen/src/isa/aarch64/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ impl AArch64Backend {
5959
domtree: &DominatorTree,
6060
ctrl_plane: &mut ControlPlane,
6161
) -> CodegenResult<(VCode<inst::Inst>, regalloc2::Output)> {
62-
let emit_info = EmitInfo::new(self.flags.clone());
62+
let emit_info = EmitInfo::new(self.flags.clone(), self.isa_flags.clone());
6363
let sigs = SigSet::new::<abi::AArch64MachineDeps>(func, &self.flags)?;
6464
let abi = abi::AArch64Callee::new(func, self, &self.isa_flags, &sigs)?;
6565
compile::compile::<AArch64Backend>(func, domtree, self, abi, emit_info, sigs, ctrl_plane)

cranelift/codegen/src/machinst/buffer.rs

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2518,6 +2518,7 @@ mod test {
25182518

25192519
use super::*;
25202520
use crate::ir::UserExternalNameRef;
2521+
use crate::isa::aarch64;
25212522
use crate::isa::aarch64::inst::{BranchTarget, CondBrKind, EmitInfo, Inst};
25222523
use crate::isa::aarch64::inst::{OperandSize, xreg};
25232524
use crate::machinst::{MachInstEmit, MachInstEmitState};
@@ -2530,9 +2531,15 @@ mod test {
25302531
BranchTarget::Label(label(n))
25312532
}
25322533

2534+
fn emit_info() -> EmitInfo {
2535+
let flags = settings::Flags::new(settings::builder());
2536+
let isa_flags = aarch64::settings::Flags::new(&flags, &aarch64::settings::builder());
2537+
EmitInfo::new(flags, isa_flags)
2538+
}
2539+
25332540
#[test]
25342541
fn test_elide_jump_to_next() {
2535-
let info = EmitInfo::new(settings::Flags::new(settings::builder()));
2542+
let info = emit_info();
25362543
let mut buf = MachBuffer::new();
25372544
let mut state = <Inst as MachInstEmit>::State::default();
25382545
let constants = Default::default();
@@ -2548,7 +2555,7 @@ mod test {
25482555

25492556
#[test]
25502557
fn test_elide_trivial_jump_blocks() {
2551-
let info = EmitInfo::new(settings::Flags::new(settings::builder()));
2558+
let info = emit_info();
25522559
let mut buf = MachBuffer::new();
25532560
let mut state = <Inst as MachInstEmit>::State::default();
25542561
let constants = Default::default();
@@ -2579,7 +2586,7 @@ mod test {
25792586

25802587
#[test]
25812588
fn test_flip_cond() {
2582-
let info = EmitInfo::new(settings::Flags::new(settings::builder()));
2589+
let info = emit_info();
25832590
let mut buf = MachBuffer::new();
25842591
let mut state = <Inst as MachInstEmit>::State::default();
25852592
let constants = Default::default();
@@ -2625,7 +2632,7 @@ mod test {
26252632

26262633
#[test]
26272634
fn test_island() {
2628-
let info = EmitInfo::new(settings::Flags::new(settings::builder()));
2635+
let info = emit_info();
26292636
let mut buf = MachBuffer::new();
26302637
let mut state = <Inst as MachInstEmit>::State::default();
26312638
let constants = Default::default();
@@ -2694,7 +2701,7 @@ mod test {
26942701

26952702
#[test]
26962703
fn test_island_backward() {
2697-
let info = EmitInfo::new(settings::Flags::new(settings::builder()));
2704+
let info = emit_info();
26982705
let mut buf = MachBuffer::new();
26992706
let mut state = <Inst as MachInstEmit>::State::default();
27002707
let constants = Default::default();
@@ -2780,7 +2787,7 @@ mod test {
27802787
// label7:
27812788
// ret
27822789

2783-
let info = EmitInfo::new(settings::Flags::new(settings::builder()));
2790+
let info = emit_info();
27842791
let mut buf = MachBuffer::new();
27852792
let mut state = <Inst as MachInstEmit>::State::default();
27862793
let constants = Default::default();
@@ -2857,7 +2864,7 @@ mod test {
28572864
//
28582865
// label0, label1, ..., label4:
28592866
// b label0
2860-
let info = EmitInfo::new(settings::Flags::new(settings::builder()));
2867+
let info = emit_info();
28612868
let mut buf = MachBuffer::new();
28622869
let mut state = <Inst as MachInstEmit>::State::default();
28632870
let constants = Default::default();

cranelift/filetests/filetests/isa/aarch64/bti.clif

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -57,31 +57,30 @@ block5(v5: i32):
5757
; hint #0x22
5858
; block1: ; offset 0x4
5959
; cmp w0, #3
60-
; b.hs #0x54
60+
; b.hs #0x50
6161
; csel x11, xzr, x0, hs
62-
; csdb
63-
; adr x10, #0x24
62+
; adr x10, #0x20
6463
; ldrsw x11, [x10, w11, uxtw #2]
6564
; add x10, x10, x11
6665
; br x10
6766
; .byte 0x24, 0x00, 0x00, 0x00
6867
; .byte 0x18, 0x00, 0x00, 0x00
6968
; .byte 0x0c, 0x00, 0x00, 0x00
70-
; block2: ; offset 0x30
69+
; block2: ; offset 0x2c
7170
; hint #0x24
7271
; mov w5, #3
73-
; b #0x58
74-
; block3: ; offset 0x3c
72+
; b #0x54
73+
; block3: ; offset 0x38
7574
; hint #0x24
7675
; mov w5, #2
77-
; b #0x58
78-
; block4: ; offset 0x48
76+
; b #0x54
77+
; block4: ; offset 0x44
7978
; hint #0x24
8079
; mov w5, #1
81-
; b #0x58
82-
; block5: ; offset 0x54
80+
; b #0x54
81+
; block5: ; offset 0x50
8382
; mov w5, #4
84-
; block6: ; offset 0x58
83+
; block6: ; offset 0x54
8584
; add w0, w0, w5
8685
; ret
8786

@@ -124,20 +123,19 @@ block2:
124123
; ldr x5, [x0]
125124
; mov x8, x5
126125
; cmp w0, #1
127-
; b.hs #0x40
126+
; b.hs #0x3c
128127
; csel x7, xzr, x0, hs
129-
; csdb
130-
; adr x6, #0x2c
128+
; adr x6, #0x28
131129
; ldrsw x7, [x6, w7, uxtw #2]
132130
; add x6, x6, x7
133131
; br x6
134132
; .byte 0x04, 0x00, 0x00, 0x00
135-
; block2: ; offset 0x30
133+
; block2: ; offset 0x2c
136134
; hint #0x24
137135
; mov x0, x8
138136
; add x0, x0, #0x2a
139137
; ret
140-
; block3: ; offset 0x40
138+
; block3: ; offset 0x3c
141139
; mov x0, x8
142140
; ret
143141

0 commit comments

Comments
 (0)