Skip to content

Fix long-range (non-colocated) aarch64 calls to not use Arm64Call reloc, and fix simplejit to use new long-distance call. #1570

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions cranelift/codegen/src/ir/extfunc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

use crate::ir::{ArgumentLoc, ExternalName, SigRef, Type};
use crate::isa::{CallConv, RegInfo, RegUnit};
use crate::machinst::RelocDistance;
use alloc::vec::Vec;
use core::fmt;
use core::str::FromStr;
Expand Down Expand Up @@ -366,6 +367,16 @@ pub struct ExtFuncData {
/// Will this function be defined nearby, such that it will always be a certain distance away,
/// after linking? If so, references to it can avoid going through a GOT or PLT. Note that
/// symbols meant to be preemptible cannot be considered colocated.
///
/// If `true`, some backends may use relocation forms that have limited range. The exact
/// distance depends on the code model in use. Currently on AArch64, for example, Cranelift
/// uses a custom code model supporting up to +/- 128MB displacements. If it is unknown how
/// far away the target will be, it is best not to set the `colocated` flag; in general, this
/// flag is best used when the target is known to be in the same unit of code generation, such
/// as a Wasm module.
///
/// See the documentation for [`RelocDistance`](machinst::RelocDistance) for more details. A
/// `colocated` flag value of `true` implies `RelocDistance::Near`.
pub colocated: bool,
}

Expand All @@ -378,6 +389,17 @@ impl fmt::Display for ExtFuncData {
}
}

impl ExtFuncData {
/// Return an estimate of the distance to the referred-to function symbol.
pub fn reloc_distance(&self) -> RelocDistance {
if self.colocated {
RelocDistance::Near
} else {
RelocDistance::Far
}
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
19 changes: 19 additions & 0 deletions cranelift/codegen/src/ir/globalvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use crate::ir::immediates::{Imm64, Offset32};
use crate::ir::{ExternalName, GlobalValue, Type};
use crate::isa::TargetIsa;
use crate::machinst::RelocDistance;
use core::fmt;

/// Information about a global value declaration.
Expand Down Expand Up @@ -62,6 +63,10 @@ pub enum GlobalValueData {
/// Will this symbol be defined nearby, such that it will always be a certain distance
/// away, after linking? If so, references to it can avoid going through a GOT. Note that
/// symbols meant to be preemptible cannot be colocated.
///
/// If `true`, some backends may use relocation forms that have limited range: for example,
/// a +/- 2^27-byte range on AArch64. See the documentation for
/// [`RelocDistance`](machinst::RelocDistance) for more details.
colocated: bool,

/// Does this symbol refer to a thread local storage value?
Expand All @@ -85,6 +90,20 @@ impl GlobalValueData {
Self::IAddImm { global_type, .. } | Self::Load { global_type, .. } => global_type,
}
}

/// If this global references a symbol, return an estimate of the relocation distance,
/// based on the `colocated` flag.
pub fn maybe_reloc_distance(&self) -> Option<RelocDistance> {
match self {
&GlobalValueData::Symbol {
colocated: true, ..
} => Some(RelocDistance::Near),
&GlobalValueData::Symbol {
colocated: false, ..
} => Some(RelocDistance::Far),
_ => None,
}
}
}

impl fmt::Display for GlobalValueData {
Expand Down
22 changes: 19 additions & 3 deletions cranelift/codegen/src/isa/aarch64/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1061,7 +1061,7 @@ impl ABIBody for AArch64ABIBody {
}

enum CallDest {
ExtName(ir::ExternalName),
ExtName(ir::ExternalName, RelocDistance),
Reg(Reg),
}

Expand Down Expand Up @@ -1102,6 +1102,7 @@ impl AArch64ABICall {
pub fn from_func(
sig: &ir::Signature,
extname: &ir::ExternalName,
dist: RelocDistance,
loc: ir::SourceLoc,
) -> AArch64ABICall {
let sig = ABISig::from_func_sig(sig);
Expand All @@ -1110,7 +1111,7 @@ impl AArch64ABICall {
sig,
uses,
defs,
dest: CallDest::ExtName(extname.clone()),
dest: CallDest::ExtName(extname.clone(), dist),
loc,
opcode: ir::Opcode::Call,
}
Expand Down Expand Up @@ -1207,13 +1208,28 @@ impl ABICall for AArch64ABICall {
fn gen_call(&self) -> Vec<Inst> {
let (uses, defs) = (self.uses.clone(), self.defs.clone());
match &self.dest {
&CallDest::ExtName(ref name) => vec![Inst::Call {
&CallDest::ExtName(ref name, RelocDistance::Near) => vec![Inst::Call {
dest: name.clone(),
uses,
defs,
loc: self.loc,
opcode: self.opcode,
}],
&CallDest::ExtName(ref name, RelocDistance::Far) => vec![
Inst::LoadExtName {
rd: writable_spilltmp_reg(),
name: name.clone(),
offset: 0,
srcloc: self.loc,
},
Inst::CallInd {
rn: spilltmp_reg(),
uses,
defs,
loc: self.loc,
opcode: self.opcode,
},
],
&CallDest::Reg(reg) => vec![Inst::CallInd {
rn: reg,
uses,
Expand Down
5 changes: 4 additions & 1 deletion cranelift/codegen/src/isa/aarch64/inst/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -612,7 +612,10 @@ pub enum Inst {
cond: Cond,
},

/// A machine call instruction.
/// A machine call instruction. N.B.: this allows only a +/- 128MB offset (it uses a relocation
/// of type `Reloc::Arm64Call`); if the destination distance is not `RelocDistance::Near`, the
/// code should use a `LoadExtName` / `CallInd` sequence instead, allowing an arbitrary 64-bit
/// target.
Call {
dest: ExternalName,
uses: Set<Reg>,
Expand Down
12 changes: 8 additions & 4 deletions cranelift/codegen/src/isa/aarch64/lower_inst.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1233,7 +1233,8 @@ pub(crate) fn lower_insn_to_regs<C: LowerCtx<I = Inst>>(ctx: &mut C, insn: IRIns

Opcode::FuncAddr => {
let rd = output_to_reg(ctx, outputs[0]);
let extname = ctx.call_target(insn).unwrap().clone();
let (extname, _) = ctx.call_target(insn).unwrap();
let extname = extname.clone();
let loc = ctx.srcloc(insn);
ctx.emit(Inst::LoadExtName {
rd,
Expand All @@ -1249,7 +1250,7 @@ pub(crate) fn lower_insn_to_regs<C: LowerCtx<I = Inst>>(ctx: &mut C, insn: IRIns

Opcode::SymbolValue => {
let rd = output_to_reg(ctx, outputs[0]);
let (extname, offset) = ctx.symbol_value(insn).unwrap();
let (extname, _, offset) = ctx.symbol_value(insn).unwrap();
let extname = extname.clone();
let loc = ctx.srcloc(insn);
ctx.emit(Inst::LoadExtName {
Expand All @@ -1264,12 +1265,15 @@ pub(crate) fn lower_insn_to_regs<C: LowerCtx<I = Inst>>(ctx: &mut C, insn: IRIns
let loc = ctx.srcloc(insn);
let (abi, inputs) = match op {
Opcode::Call => {
let extname = ctx.call_target(insn).unwrap();
let (extname, dist) = ctx.call_target(insn).unwrap();
let extname = extname.clone();
let sig = ctx.call_sig(insn).unwrap();
assert!(inputs.len() == sig.params.len());
assert!(outputs.len() == sig.returns.len());
(AArch64ABICall::from_func(sig, &extname, loc), &inputs[..])
(
AArch64ABICall::from_func(sig, &extname, dist, loc),
&inputs[..],
)
}
Opcode::CallIndirect => {
let ptr = input_to_reg(ctx, inputs[0], NarrowValueMode::ZeroExtend64);
Expand Down
40 changes: 30 additions & 10 deletions cranelift/codegen/src/machinst/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,15 @@ pub trait LowerCtx {
fn bb_param(&self, bb: Block, idx: usize) -> Reg;
/// Get the register for a return value.
fn retval(&self, idx: usize) -> Writable<Reg>;
/// Get the target for a call instruction, as an `ExternalName`.
fn call_target<'b>(&'b self, ir_inst: Inst) -> Option<&'b ExternalName>;
/// Get the target for a call instruction, as an `ExternalName`. Returns a tuple
/// providing this name and the "relocation distance", i.e., whether the backend
/// can assume the target will be "nearby" (within some small offset) or an
/// arbitrary address. (This comes from the `colocated` bit in the CLIF.)
fn call_target<'b>(&'b self, ir_inst: Inst) -> Option<(&'b ExternalName, RelocDistance)>;
/// Get the signature for a call or call-indirect instruction.
fn call_sig<'b>(&'b self, ir_inst: Inst) -> Option<&'b Signature>;
/// Get the symbol name and offset for a symbol_value instruction.
fn symbol_value<'b>(&'b self, ir_inst: Inst) -> Option<(&'b ExternalName, i64)>;
/// Get the symbol name, relocation distance estimate, and offset for a symbol_value instruction.
fn symbol_value<'b>(&'b self, ir_inst: Inst) -> Option<(&'b ExternalName, RelocDistance, i64)>;
/// Returns the memory flags of a given memory access.
fn memflags(&self, ir_inst: Inst) -> Option<MemFlags>;
/// Get the source location for a given instruction.
Expand Down Expand Up @@ -122,6 +125,18 @@ pub struct Lower<'func, I: VCodeInst> {
next_vreg: u32,
}

/// Notion of "relocation distance". This gives an estimate of how far away a symbol will be from a
/// reference.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum RelocDistance {
/// Target of relocation is "nearby". The threshold for this is fuzzy but should be interpreted
/// as approximately "within the compiled output of one module"; e.g., within AArch64's +/-
/// 128MB offset. If unsure, use `Far` instead.
Near,
/// Target of relocation could be anywhere in the address space.
Far,
}

fn alloc_vreg(
value_regs: &mut SecondaryMap<Value, Reg>,
regclass: RegClass,
Expand Down Expand Up @@ -647,13 +662,17 @@ impl<'func, I: VCodeInst> LowerCtx for Lower<'func, I> {
Writable::from_reg(self.retval_regs[idx].0)
}

/// Get the target for a call instruction, as an `ExternalName`.
fn call_target<'b>(&'b self, ir_inst: Inst) -> Option<&'b ExternalName> {
/// Get the target for a call instruction, as an `ExternalName`. Returns a tuple
/// providing this name and the "relocation distance", i.e., whether the backend
/// can assume the target will be "nearby" (within some small offset) or an
/// arbitrary address. (This comes from the `colocated` bit in the CLIF.)
fn call_target<'b>(&'b self, ir_inst: Inst) -> Option<(&'b ExternalName, RelocDistance)> {
match &self.f.dfg[ir_inst] {
&InstructionData::Call { func_ref, .. }
| &InstructionData::FuncAddr { func_ref, .. } => {
let funcdata = &self.f.dfg.ext_funcs[func_ref];
Some(&funcdata.name)
let dist = funcdata.reloc_distance();
Some((&funcdata.name, dist))
}
_ => None,
}
Expand All @@ -670,8 +689,8 @@ impl<'func, I: VCodeInst> LowerCtx for Lower<'func, I> {
}
}

/// Get the symbol name and offset for a symbol_value instruction.
fn symbol_value<'b>(&'b self, ir_inst: Inst) -> Option<(&'b ExternalName, i64)> {
/// Get the symbol name, relocation distance estimate, and offset for a symbol_value instruction.
fn symbol_value<'b>(&'b self, ir_inst: Inst) -> Option<(&'b ExternalName, RelocDistance, i64)> {
match &self.f.dfg[ir_inst] {
&InstructionData::UnaryGlobalValue { global_value, .. } => {
let gvdata = &self.f.global_values[global_value];
Expand All @@ -682,7 +701,8 @@ impl<'func, I: VCodeInst> LowerCtx for Lower<'func, I> {
..
} => {
let offset = offset.bits();
Some((name, offset))
let dist = gvdata.maybe_reloc_distance().unwrap();
Some((name, dist, offset))
}
_ => None,
}
Expand Down
3 changes: 2 additions & 1 deletion cranelift/filetests/filetests/vcode/aarch64/call.clif
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ block0(v0: i64):

; check: stp fp, lr, [sp, #-16]!
; nextln: mov fp, sp
; nextln: bl 0
; nextln: ldr x15, 8 ; b 12 ; data
; nextln: blr x15
; nextln: mov sp, fp
; nextln: ldp fp, lr, [sp], #16
; nextln: ret
6 changes: 4 additions & 2 deletions cranelift/filetests/filetests/vcode/aarch64/stack-limit.clif
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ block0(v0: i64):
; nextln: subs xzr, sp, x0
; nextln: b.hs 8
; nextln: udf
; nextln: bl 0
; nextln: ldr x15
; nextln: blr x15
; nextln: mov sp, fp
; nextln: ldp fp, lr, [sp], #16
; nextln: ret
Expand All @@ -68,7 +69,8 @@ block0(v0: i64):
; nextln: subs xzr, sp, x15
; nextln: b.hs 8
; nextln: udf
; nextln: bl 0
; nextln: ldr x15
; nextln: blr x15
; nextln: mov sp, fp
; nextln: ldp fp, lr, [sp], #16
; nextln: ret
Expand Down
7 changes: 6 additions & 1 deletion cranelift/simplejit/src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use cranelift_codegen::binemit::{
Addend, CodeOffset, Reloc, RelocSink, Stackmap, StackmapSink, TrapSink,
};
use cranelift_codegen::isa::TargetIsa;
use cranelift_codegen::settings::Configurable;
use cranelift_codegen::{self, ir, settings};
use cranelift_module::{
Backend, DataContext, DataDescription, DataId, FuncId, Init, Linkage, ModuleNamespace,
Expand Down Expand Up @@ -40,7 +41,11 @@ impl SimpleJITBuilder {
/// floating point instructions, and for stack probes. If you don't know what to use for this
/// argument, use `cranelift_module::default_libcall_names()`.
pub fn new(libcall_names: Box<dyn Fn(ir::LibCall) -> String>) -> Self {
let flag_builder = settings::builder();
let mut flag_builder = settings::builder();
// On at least AArch64, "colocated" calls use shorter-range relocations,
// which might not reach all definitions; we can't handle that here, so
// we require long-range relocation types.
flag_builder.set("use_colocated_libcalls", "false").unwrap();
let isa_builder = cranelift_native::builder().unwrap_or_else(|msg| {
panic!("host machine is not supported: {}", msg);
});
Expand Down
1 change: 0 additions & 1 deletion cranelift/simplejit/tests/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ fn switch_error() {
}

#[test]
#[cfg_attr(target_arch = "aarch64", should_panic)] // FIXME(#1521)
fn libcall_function() {
let mut module: Module<SimpleJITBackend> =
Module::new(SimpleJITBuilder::new(default_libcall_names()));
Expand Down