Skip to content

Commit 3fe9c3c

Browse files
pnodetbjorn3
andauthored
fix: accurate leaf detection (#11581)
* feat: add is_call() method to MachInst trait and VCode analysis Add is_call() method to MachInst trait to enable accurate leaf function detection during register allocation. Update VCode compute_clobbers() to return (clobbers, is_leaf) tuple by analyzing actual call instructions in machine code. * feat: implement is_call() method across all architectures Implement is_call() method for all architecture-specific MachInst implementations: - x64: Detects CallKnown, CallUnknown, ReturnCall variants, and TLS calls (ElfTlsGetAddr, MachOTlsGetAddr) - aarch64: Detects Call, CallInd, ReturnCall variants, and TLS calls (ElfTlsGetAddr, MachOTlsGetAddr) - riscv64: Detects Call, CallInd, ReturnCall variants, and ElfTlsGetAddr - s390x: Detects CallKnown, CallUnknown, ReturnCall variants - pulley: Detects Call, CallIndirect, ReturnCall variants Co-authored-by: bjorn3 <[email protected]> * feat: improve leaf function detection and pass is_leaf to FrameLayout * test: add filetests for leaf detection * test: update expected outputs for accurate leaf function detection * test(riscv64): update filetests output --------- Co-authored-by: bjorn3 <[email protected]>
1 parent 3fa26f6 commit 3fe9c3c

File tree

23 files changed

+891
-111
lines changed

23 files changed

+891
-111
lines changed

cranelift/codegen/src/ir/function.rs

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -320,24 +320,6 @@ impl FunctionStencil {
320320
})
321321
}
322322

323-
/// Returns true if the function is function that doesn't call any other functions. This is not
324-
/// to be confused with a "leaf function" in Windows terminology.
325-
pub fn is_leaf(&self) -> bool {
326-
// Conservative result: if there's at least one function signature referenced in this
327-
// function, assume it is not a leaf.
328-
let has_signatures = !self.dfg.signatures.is_empty();
329-
330-
// Under some TLS models, retrieving the address of a TLS variable requires calling a
331-
// function. Conservatively assume that any function that references a tls global value
332-
// is not a leaf.
333-
let has_tls = self.global_values.values().any(|gv| match gv {
334-
GlobalValueData::Symbol { tls, .. } => *tls,
335-
_ => false,
336-
});
337-
338-
!has_signatures && !has_tls
339-
}
340-
341323
/// Replace the `dst` instruction's data with the `src` instruction's data
342324
/// and then remove `src`.
343325
///

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1167,6 +1167,7 @@ impl ABIMachineSpec for AArch64MachineDeps {
11671167
stackslots_size,
11681168
outgoing_args_size,
11691169
clobbered_callee_saves: regs,
1170+
is_leaf,
11701171
}
11711172
}
11721173

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1000,6 +1000,18 @@ impl MachInst for Inst {
10001000
}
10011001
}
10021002

1003+
fn is_call(&self) -> bool {
1004+
match self {
1005+
Inst::Call { .. }
1006+
| Inst::CallInd { .. }
1007+
| Inst::ReturnCall { .. }
1008+
| Inst::ReturnCallInd { .. }
1009+
| Inst::ElfTlsGetAddr { .. }
1010+
| Inst::MachOTlsGetAddr { .. } => true,
1011+
_ => false,
1012+
}
1013+
}
1014+
10031015
fn is_term(&self) -> MachTerminator {
10041016
match self {
10051017
&Inst::Rets { .. } => MachTerminator::Ret,

cranelift/codegen/src/isa/pulley_shared/abi.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -537,6 +537,7 @@ where
537537
stackslots_size,
538538
outgoing_args_size,
539539
clobbered_callee_saves: regs,
540+
is_leaf,
540541
}
541542
}
542543

cranelift/codegen/src/isa/pulley_shared/inst/mod.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,17 @@ where
490490
todo!()
491491
}
492492

493+
fn is_call(&self) -> bool {
494+
match &self.inst {
495+
Inst::Call { .. }
496+
| Inst::IndirectCall { .. }
497+
| Inst::IndirectCallHost { .. }
498+
| Inst::ReturnCall { .. }
499+
| Inst::ReturnIndirectCall { .. } => true,
500+
_ => false,
501+
}
502+
}
503+
493504
fn gen_move(to_reg: Writable<Reg>, from_reg: Reg, ty: Type) -> Self {
494505
match ty {
495506
ir::types::I8 | ir::types::I16 | ir::types::I32 | ir::types::I64 => RawInst::Xmov {

cranelift/codegen/src/isa/riscv64/abi.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -671,6 +671,7 @@ impl ABIMachineSpec for Riscv64MachineDeps {
671671
stackslots_size,
672672
outgoing_args_size,
673673
clobbered_callee_saves: regs,
674+
is_leaf,
674675
}
675676
}
676677

cranelift/codegen/src/isa/riscv64/inst/mod.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -758,6 +758,17 @@ impl MachInst for Inst {
758758
}
759759
}
760760

761+
fn is_call(&self) -> bool {
762+
match self {
763+
Inst::Call { .. }
764+
| Inst::CallInd { .. }
765+
| Inst::ReturnCall { .. }
766+
| Inst::ReturnCallInd { .. }
767+
| Inst::ElfTlsGetAddr { .. } => true,
768+
_ => false,
769+
}
770+
}
771+
761772
fn is_term(&self) -> MachTerminator {
762773
match self {
763774
&Inst::Jal { .. } => MachTerminator::Branch,

cranelift/codegen/src/isa/s390x/abi.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -896,7 +896,7 @@ impl ABIMachineSpec for S390xMachineDeps {
896896
flags: &settings::Flags,
897897
_sig: &Signature,
898898
regs: &[Writable<RealReg>],
899-
_is_leaf: bool,
899+
is_leaf: bool,
900900
incoming_args_size: u32,
901901
tail_args_size: u32,
902902
stackslots_size: u32,
@@ -975,6 +975,7 @@ impl ABIMachineSpec for S390xMachineDeps {
975975
stackslots_size,
976976
outgoing_args_size,
977977
clobbered_callee_saves: regs,
978+
is_leaf,
978979
}
979980
}
980981

cranelift/codegen/src/isa/s390x/inst/mod.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1122,6 +1122,13 @@ impl MachInst for Inst {
11221122
}
11231123
}
11241124

1125+
fn is_call(&self) -> bool {
1126+
match self {
1127+
Inst::Call { .. } | Inst::ReturnCall { .. } | Inst::ElfTlsGetOffset { .. } => true,
1128+
_ => false,
1129+
}
1130+
}
1131+
11251132
fn gen_move(to_reg: Writable<Reg>, from_reg: Reg, ty: Type) -> Inst {
11261133
assert!(ty.bits() <= 128);
11271134
if ty.bits() <= 32 {

cranelift/codegen/src/isa/x64/abi.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -900,7 +900,7 @@ impl ABIMachineSpec for X64ABIMachineSpec {
900900
flags: &settings::Flags,
901901
_sig: &Signature,
902902
regs: &[Writable<RealReg>],
903-
_is_leaf: bool,
903+
is_leaf: bool,
904904
incoming_args_size: u32,
905905
tail_args_size: u32,
906906
stackslots_size: u32,
@@ -947,6 +947,7 @@ impl ABIMachineSpec for X64ABIMachineSpec {
947947
stackslots_size,
948948
outgoing_args_size,
949949
clobbered_callee_saves: regs,
950+
is_leaf,
950951
}
951952
}
952953

0 commit comments

Comments
 (0)