Skip to content

Commit

Permalink
Refactor CallInfo amongst Cranelift's backends (bytecodealliance#9190)
Browse files Browse the repository at this point in the history
* Refactor backends to use the same `CallInfo`

This commit refactors the various backends of cranelift, except for
s390x, to use a shared definition of `CallInfo`. They were all already
quite similar and the main change here is to push platform-specific
pieces into the instructions outside of `CallInfo`. This is intended to
make additions to `CallInfo` easier and require less refactoring in the
future. Additionally this enables passing a `CallInfo` structure around
instead of passing around all of its components which helps reduce the
amount of arguments to various functions.

* s390x: Use the same `CallInfo` as other backends

This commit refactors s390x the same way as the previous commit to use
the shared `CallInfo` that all other backends are using. This required
more refactoring on the s390x side of things to notably extract a
dedicated pseudo-instruction for `ElfTlsGetOffset` rather than bundling
it within the `Call` instruction.

* Review comments and test fixes

* Fold `ExternalName` into `CallInfo`

As predicted instruction sizes got larger when outlining this on some
platforms so apply the same fix across all platforms by changing to
`CallInfo<T>` where the `T` will change depending on whether it's an
indirect or direct call.

* Update test expectations
  • Loading branch information
alexcrichton authored Sep 3, 2024
1 parent d6713c5 commit ae92cb4
Show file tree
Hide file tree
Showing 37 changed files with 577 additions and 866 deletions.
92 changes: 36 additions & 56 deletions cranelift/codegen/src/isa/aarch64/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1014,58 +1014,26 @@ impl ABIMachineSpec for AArch64MachineDeps {
insts
}

fn gen_call(
dest: &CallDest,
uses: CallArgList,
defs: CallRetList,
clobbers: PRegSet,
tmp: Writable<Reg>,
callee_conv: isa::CallConv,
caller_conv: isa::CallConv,
callee_pop_size: u32,
) -> SmallVec<[Inst; 2]> {
fn gen_call(dest: &CallDest, tmp: Writable<Reg>, info: CallInfo<()>) -> SmallVec<[Inst; 2]> {
let mut insts = SmallVec::new();
match &dest {
&CallDest::ExtName(ref name, RelocDistance::Near) => insts.push(Inst::Call {
info: Box::new(CallInfo {
dest: name.clone(),
uses,
defs,
clobbers,
caller_callconv: caller_conv,
callee_callconv: callee_conv,
callee_pop_size,
}),
}),
&CallDest::ExtName(ref name, RelocDistance::Near) => {
let info = Box::new(info.map(|()| name.clone()));
insts.push(Inst::Call { info });
}
&CallDest::ExtName(ref name, RelocDistance::Far) => {
insts.push(Inst::LoadExtName {
rd: tmp,
name: Box::new(name.clone()),
offset: 0,
});
insts.push(Inst::CallInd {
info: Box::new(CallIndInfo {
rn: tmp.to_reg(),
uses,
defs,
clobbers,
caller_callconv: caller_conv,
callee_callconv: callee_conv,
callee_pop_size,
}),
});
let info = Box::new(info.map(|()| tmp.to_reg()));
insts.push(Inst::CallInd { info });
}
&CallDest::Reg(reg) => {
let info = Box::new(info.map(|()| *reg));
insts.push(Inst::CallInd { info });
}
&CallDest::Reg(reg) => insts.push(Inst::CallInd {
info: Box::new(CallIndInfo {
rn: *reg,
uses,
defs,
clobbers,
caller_callconv: caller_conv,
callee_callconv: callee_conv,
callee_pop_size,
}),
}),
}

insts
Expand Down Expand Up @@ -1103,8 +1071,8 @@ impl ABIMachineSpec for AArch64MachineDeps {
],
defs: smallvec![],
clobbers: Self::get_regs_clobbered_by_call(call_conv),
caller_callconv: call_conv,
callee_callconv: call_conv,
caller_conv: call_conv,
callee_conv: call_conv,
callee_pop_size: 0,
}),
});
Expand Down Expand Up @@ -1295,16 +1263,17 @@ impl AArch64CallSite {

let dest = self.dest().clone();
let uses = self.take_uses();
let info = Box::new(ReturnCallInfo {
uses,
new_stack_arg_size,
key: select_api_key(isa_flags, isa::CallConv::Tail, true),
});
let key = select_api_key(isa_flags, isa::CallConv::Tail, true);

match dest {
CallDest::ExtName(callee, RelocDistance::Near) => {
let callee = Box::new(callee);
ctx.emit(Inst::ReturnCall { callee, info });
let info = Box::new(ReturnCallInfo {
dest: callee,
uses,
key,
new_stack_arg_size,
});
ctx.emit(Inst::ReturnCall { info });
}
CallDest::ExtName(name, RelocDistance::Far) => {
let callee = ctx.alloc_tmp(types::I64).only_reg().unwrap();
Expand All @@ -1313,12 +1282,23 @@ impl AArch64CallSite {
name: Box::new(name),
offset: 0,
});
ctx.emit(Inst::ReturnCallInd {
callee: callee.to_reg(),
info,
let info = Box::new(ReturnCallInfo {
dest: callee.to_reg(),
uses,
key,
new_stack_arg_size,
});
ctx.emit(Inst::ReturnCallInd { info });
}
CallDest::Reg(callee) => {
let info = Box::new(ReturnCallInfo {
dest: callee,
uses,
key,
new_stack_arg_size,
});
ctx.emit(Inst::ReturnCallInd { info });
}
CallDest::Reg(callee) => ctx.emit(Inst::ReturnCallInd { callee, info }),
}
}
}
Expand Down
15 changes: 5 additions & 10 deletions cranelift/codegen/src/isa/aarch64/inst.isle
Original file line number Diff line number Diff line change
Expand Up @@ -806,22 +806,16 @@
;; 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
(info BoxCallInfo))
(Call (info BoxCallInfo))

;; A machine indirect-call instruction.
(CallInd
(info BoxCallIndInfo))
(CallInd (info BoxCallIndInfo))

;; A return-call macro instruction.
(ReturnCall
(callee BoxExternalName)
(info BoxReturnCallInfo))
(ReturnCall (info BoxReturnCallInfo))

;; An indirect return-call macro instruction.
(ReturnCallInd
(callee Reg)
(info BoxReturnCallInfo))
(ReturnCallInd (info BoxReturnCallIndInfo))

;; A pseudo-instruction that captures register arguments in vregs.
(Args
Expand Down Expand Up @@ -1073,6 +1067,7 @@
(type BoxCallInfo (primitive BoxCallInfo))
(type BoxCallIndInfo (primitive BoxCallIndInfo))
(type BoxReturnCallInfo (primitive BoxReturnCallInfo))
(type BoxReturnCallIndInfo (primitive BoxReturnCallIndInfo))
(type CondBrKind (primitive CondBrKind))
(type BranchTarget (primitive BranchTarget))
(type BoxJTSequenceInfo (primitive BoxJTSequenceInfo))
Expand Down
43 changes: 14 additions & 29 deletions cranelift/codegen/src/isa/aarch64/inst/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2943,8 +2943,9 @@ impl MachInstEmit for Inst {
}
&Inst::CallInd { ref info } => {
let user_stack_map = state.take_stack_map();
let rn = info.rn;
sink.put4(0b1101011_0001_11111_000000_00000_00000 | (machreg_to_gpr(rn) << 5));
sink.put4(
0b1101011_0001_11111_000000_00000_00000 | (machreg_to_gpr(info.dest) << 5),
);
if let Some(s) = user_stack_map {
let offset = sink.cur_offset();
sink.push_user_stack_map(state, offset, s);
Expand All @@ -2959,16 +2960,13 @@ impl MachInstEmit for Inst {
}
}
}
&Inst::ReturnCall {
ref callee,
ref info,
} => {
&Inst::ReturnCall { ref info } => {
emit_return_call_common_sequence(sink, emit_info, state, info);

// Note: this is not `Inst::Jump { .. }.emit(..)` because we
// have different metadata in this case: we don't have a label
// for the target, but rather a function relocation.
sink.add_reloc(Reloc::Arm64Call, &**callee, 0);
sink.add_reloc(Reloc::Arm64Call, &info.dest, 0);
sink.put4(enc_jump26(0b000101, 0));
sink.add_call_site();

Expand All @@ -2977,11 +2975,11 @@ impl MachInstEmit for Inst {
// in this case.
start_off = sink.cur_offset();
}
&Inst::ReturnCallInd { callee, ref info } => {
&Inst::ReturnCallInd { ref info } => {
emit_return_call_common_sequence(sink, emit_info, state, info);

Inst::IndirectBr {
rn: callee,
rn: info.dest,
targets: vec![],
}
.emit(sink, emit_info, state);
Expand Down Expand Up @@ -3372,15 +3370,7 @@ impl MachInstEmit for Inst {
// blr tmp
sink.add_reloc(Reloc::Aarch64TlsDescCall, &**symbol, 0);
Inst::CallInd {
info: crate::isa::Box::new(CallIndInfo {
rn: tmp.to_reg(),
uses: smallvec![],
defs: smallvec![],
clobbers: PRegSet::empty(),
caller_callconv: CallConv::SystemV,
callee_callconv: CallConv::SystemV,
callee_pop_size: 0,
}),
info: crate::isa::Box::new(CallInfo::empty(tmp.to_reg(), CallConv::SystemV)),
}
.emit(sink, emit_info, state);

Expand Down Expand Up @@ -3432,15 +3422,10 @@ impl MachInstEmit for Inst {

// call function pointer in temp register
Inst::CallInd {
info: crate::isa::Box::new(CallIndInfo {
rn: rtmp.to_reg(),
uses: smallvec![],
defs: smallvec![],
clobbers: PRegSet::empty(),
caller_callconv: CallConv::AppleAarch64,
callee_callconv: CallConv::AppleAarch64,
callee_pop_size: 0,
}),
info: crate::isa::Box::new(CallInfo::empty(
rtmp.to_reg(),
CallConv::AppleAarch64,
)),
}
.emit(sink, emit_info, state);
}
Expand Down Expand Up @@ -3533,11 +3518,11 @@ impl MachInstEmit for Inst {
}
}

fn emit_return_call_common_sequence(
fn emit_return_call_common_sequence<T>(
sink: &mut MachBuffer<Inst>,
emit_info: &EmitInfo,
state: &mut EmitState,
info: &ReturnCallInfo,
info: &ReturnCallInfo<T>,
) {
for inst in
AArch64MachineDeps::gen_clobber_restore(CallConv::Tail, &emit_info.0, state.frame_layout())
Expand Down
25 changes: 6 additions & 19 deletions cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::ir::types::*;
use crate::ir::TrapCode;
use crate::ir::{ExternalName, TrapCode};
use crate::isa::aarch64::inst::*;

use alloc::boxed::Box;
Expand Down Expand Up @@ -6056,31 +6056,18 @@ fn test_aarch64_binemit() {

insns.push((
Inst::Call {
info: Box::new(CallInfo {
dest: ExternalName::testcase("test0"),
uses: smallvec![],
defs: smallvec![],
clobbers: PRegSet::empty(),
caller_callconv: CallConv::SystemV,
callee_callconv: CallConv::SystemV,
callee_pop_size: 0,
}),
info: Box::new(CallInfo::empty(
ExternalName::testcase("test0"),
CallConv::SystemV,
)),
},
"00000094",
"bl 0",
));

insns.push((
Inst::CallInd {
info: Box::new(CallIndInfo {
rn: xreg(10),
uses: smallvec![],
defs: smallvec![],
clobbers: PRegSet::empty(),
caller_callconv: CallConv::SystemV,
callee_callconv: CallConv::SystemV,
callee_pop_size: 0,
}),
info: Box::new(CallInfo::empty(xreg(10), CallConv::SystemV)),
},
"40013FD6",
"blr x10",
Expand Down
Loading

0 comments on commit ae92cb4

Please sign in to comment.