Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Make the Tail call conv follow the system call conv for the return area ptr #9287

Merged
merged 2 commits into from
Sep 20, 2024
+605 −656
Merged

Make the Tail call conv follow the system call conv for the return area ptr #9287

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
439fa7c
Make the Tail call conv follow the system call conv for the return ar…
bjorn3 Sep 19, 2024
7e163b2
Fix review comments
bjorn3 Sep 20, 2024
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
63 changes: 29 additions & 34 deletions cranelift/codegen/src/isa/aarch64/abi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,33 @@ impl ABIMachineSpec for AArch64MachineDeps {
let max_per_class_reg_vals = 8; // x0-x7 and v0-v7
let mut remaining_reg_vals = 16;

let ret_area_ptr = if add_ret_area_ptr {
debug_assert_eq!(args_or_rets, ArgsOrRets::Args);
if call_conv != isa::CallConv::Winch {
// In the AAPCS64 calling convention the return area pointer is
// stored in x8.
Some(ABIArg::reg(
xreg(8).to_real_reg().unwrap(),
I64,
ir::ArgumentExtension::None,
ir::ArgumentPurpose::Normal,
))
} else {
// Use x0 for the return area pointer in the Winch calling convention
// to simplify the ABI handling code in Winch by avoiding an AArch64
// special case to assign it to x8.
next_xreg += 1;
Some(ABIArg::reg(
xreg(0).to_real_reg().unwrap(),
I64,
ir::ArgumentExtension::None,
ir::ArgumentPurpose::Normal,
))
}
} else {
None
};

for (i, param) in params.into_iter().enumerate() {
if is_apple_cc && param.value_type == types::F128 && !flags.enable_llvm_abi_extensions()
{
Expand Down Expand Up @@ -349,40 +376,8 @@ impl ABIMachineSpec for AArch64MachineDeps {
next_stack += size;
}

let extra_arg = if add_ret_area_ptr {
debug_assert!(args_or_rets == ArgsOrRets::Args);
if call_conv != isa::CallConv::Tail && call_conv != isa::CallConv::Winch {
args.push_non_formal(ABIArg::reg(
xreg(8).to_real_reg().unwrap(),
I64,
ir::ArgumentExtension::None,
ir::ArgumentPurpose::Normal,
));
} else if call_conv == isa::CallConv::Tail {
args.push_non_formal(ABIArg::reg(
xreg_preg(0).into(),
I64,
ir::ArgumentExtension::None,
ir::ArgumentPurpose::Normal,
));
} else {
if next_xreg < max_per_class_reg_vals && remaining_reg_vals > 0 {
args.push_non_formal(ABIArg::reg(
xreg(next_xreg).to_real_reg().unwrap(),
I64,
ir::ArgumentExtension::None,
ir::ArgumentPurpose::Normal,
));
} else {
args.push_non_formal(ABIArg::stack(
next_stack as i64,
I64,
ir::ArgumentExtension::None,
ir::ArgumentPurpose::Normal,
));
next_stack += 8;
}
}
let extra_arg = if let Some(ret_area_ptr) = ret_area_ptr {
args.push_non_formal(ret_area_ptr);
Some(args.args().len() - 1)
} else {
None
Expand Down
35 changes: 15 additions & 20 deletions cranelift/codegen/src/isa/pulley_shared/abi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,19 @@ where
let mut next_v_reg = 0;
let mut next_stack: u32 = 0;

let ret_area_ptr = if add_ret_area_ptr {
debug_assert_eq!(args_or_rets, ArgsOrRets::Args);
next_x_reg += 1;
Some(ABIArg::reg(
x_reg(next_x_reg - 1).to_real_reg().unwrap(),
I64,
ir::ArgumentExtension::None,
ir::ArgumentPurpose::Normal,
))
} else {
None
};

for param in params {
// Find the regclass(es) of the register(s) used to store a value of
// this type.
Expand Down Expand Up @@ -125,26 +138,8 @@ where
});
}

let pos = if add_ret_area_ptr {
assert!(ArgsOrRets::Args == args_or_rets);
if next_x_reg <= x_end {
let arg = ABIArg::reg(
x_reg(next_x_reg).to_real_reg().unwrap(),
I64,
ir::ArgumentExtension::None,
ir::ArgumentPurpose::Normal,
);
args.push_non_formal(arg);
} else {
let arg = ABIArg::stack(
next_stack as i64,
I64,
ir::ArgumentExtension::None,
ir::ArgumentPurpose::Normal,
);
args.push_non_formal(arg);
next_stack += 8;
}
let pos = if let Some(ret_area_ptr) = ret_area_ptr {
args.push_non_formal(ret_area_ptr);
Some(args.args().len() - 1)
} else {
None
Expand Down
46 changes: 13 additions & 33 deletions cranelift/codegen/src/isa/riscv64/abi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,18 @@ impl ABIMachineSpec for Riscv64MachineDeps {
// Stack space.
let mut next_stack: u32 = 0;

// In the SystemV ABI, the return area pointer is the first argument,
// so we need to leave room for it if required.
if add_ret_area_ptr && call_conv != CallConv::Tail && call_conv != CallConv::Winch {
let ret_area_ptr = if add_ret_area_ptr {
assert!(ArgsOrRets::Args == args_or_rets);
next_x_reg += 1;
}
Some(ABIArg::reg(
x_reg(x_start).to_real_reg().unwrap(),
I64,
ir::ArgumentExtension::None,
ir::ArgumentPurpose::Normal,
))
Comment on lines 116 to +122
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel similarly here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This really needs to be the first argument register. If next_x_reg - 1 is not x_start at this point, that is a bug.

} else {
None
};

for param in params {
if let ir::ArgumentPurpose::StructArgument(_) = param.purpose {
Expand Down Expand Up @@ -167,35 +174,8 @@ impl ABIMachineSpec for Riscv64MachineDeps {
purpose: param.purpose,
});
}
let pos: Option<usize> = if add_ret_area_ptr {
assert!(ArgsOrRets::Args == args_or_rets);
if call_conv != CallConv::Tail && call_conv != CallConv::Winch {
// In the SystemV ABI, the return area pointer is the first argument.
let arg = ABIArg::reg(
x_reg(x_start).to_real_reg().unwrap(),
I64,
ir::ArgumentExtension::None,
ir::ArgumentPurpose::Normal,
);
args.push_non_formal(arg);
} else if next_x_reg <= x_end {
let arg = ABIArg::reg(
x_reg(next_x_reg).to_real_reg().unwrap(),
I64,
ir::ArgumentExtension::None,
ir::ArgumentPurpose::Normal,
);
args.push_non_formal(arg);
} else {
let arg = ABIArg::stack(
next_stack as i64,
I64,
ir::ArgumentExtension::None,
ir::ArgumentPurpose::Normal,
);
args.push_non_formal(arg);
next_stack += 8;
}
let pos = if let Some(ret_area_ptr) = ret_area_ptr {
args.push_non_formal(ret_area_ptr);
Some(args.args().len() - 1)
} else {
None
Expand Down
39 changes: 16 additions & 23 deletions cranelift/codegen/src/isa/s390x/abi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,11 +315,21 @@ impl ABIMachineSpec for S390xMachineDeps {
next_stack = REG_SAVE_AREA_SIZE;
}

// In the SystemV ABI, the return area pointer is the first argument,
// so we need to leave room for it if required.
if add_ret_area_ptr {
let ret_area_ptr = if add_ret_area_ptr {
debug_assert_eq!(args_or_rets, ArgsOrRets::Args);
next_gpr += 1;
}
Some(ABIArg::reg(
get_intreg_for_arg(call_conv, 0)
.unwrap()
.to_real_reg()
.unwrap(),
types::I64,
ir::ArgumentExtension::None,
ir::ArgumentPurpose::Normal,
))
} else {
None
};

for mut param in params.into_iter().copied() {
if let ir::ArgumentPurpose::StructArgument(_) = param.purpose {
Expand Down Expand Up @@ -421,25 +431,8 @@ impl ABIMachineSpec for S390xMachineDeps {

next_stack = align_to(next_stack, 8);

let extra_arg = if add_ret_area_ptr {
debug_assert!(args_or_rets == ArgsOrRets::Args);
// The return pointer is passed as first argument.
if let Some(reg) = get_intreg_for_arg(call_conv, 0) {
args.push(ABIArg::reg(
reg.to_real_reg().unwrap(),
types::I64,
ir::ArgumentExtension::None,
ir::ArgumentPurpose::Normal,
));
} else {
args.push(ABIArg::stack(
next_stack as i64,
types::I64,
ir::ArgumentExtension::None,
ir::ArgumentPurpose::Normal,
));
next_stack += 8;
}
let extra_arg = if let Some(ret_area_ptr) = ret_area_ptr {
args.push_non_formal(ret_area_ptr);
Some(args.args().len() - 1)
} else {
None
Expand Down
55 changes: 19 additions & 36 deletions cranelift/codegen/src/isa/x64/abi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,24 @@ impl ABIMachineSpec for X64ABIMachineSpec {
next_stack = 32;
}

// In the SystemV and WindowsFastcall ABIs, the return area pointer is the first argument,
// so we need to leave room for it if required.
if add_ret_area_ptr && call_conv != CallConv::Tail && call_conv != CallConv::Winch {
let ret_area_ptr = if add_ret_area_ptr {
debug_assert_eq!(args_or_rets, ArgsOrRets::Args);
next_gpr += 1;
next_param_idx += 1;
}
// In the SystemV and WindowsFastcall ABIs, the return area pointer is the first
// argument. For the Tail and Winch ABIs we do the same for simplicity sake.
Some(ABIArg::reg(
get_intreg_for_arg(call_conv, 0, 0)
.unwrap()
.to_real_reg()
.unwrap(),
types::I64,
ir::ArgumentExtension::None,
ir::ArgumentPurpose::Normal,
))
} else {
None
};

// If any param uses extension, the winch calling convention will not pack its results
// on the stack and will instead align them to 8-byte boundaries the same way that all the
Expand Down Expand Up @@ -368,37 +380,8 @@ impl ABIMachineSpec for X64ABIMachineSpec {
}
}
}

let extra_arg = if add_ret_area_ptr {
debug_assert!(args_or_rets == ArgsOrRets::Args);
if call_conv != CallConv::Tail && call_conv != CallConv::Winch {
// In the SystemV and WindowsFastcall ABIs, the return area pointer is the first
// argument.
args.push_non_formal(ABIArg::reg(
get_intreg_for_arg(call_conv, 0, 0)
.unwrap()
.to_real_reg()
.unwrap(),
types::I64,
ir::ArgumentExtension::None,
ir::ArgumentPurpose::Normal,
));
} else if let Some(reg) = get_intreg_for_arg(call_conv, next_gpr, next_param_idx) {
args.push_non_formal(ABIArg::reg(
reg.to_real_reg().unwrap(),
types::I64,
ir::ArgumentExtension::None,
ir::ArgumentPurpose::Normal,
));
} else {
args.push_non_formal(ABIArg::stack(
next_stack as i64,
types::I64,
ir::ArgumentExtension::None,
ir::ArgumentPurpose::Normal,
));
next_stack += 8;
}
let extra_arg_idx = if let Some(ret_area_ptr) = ret_area_ptr {
args.push_non_formal(ret_area_ptr);
Some(args.args().len() - 1)
} else {
None
Expand All @@ -412,7 +395,7 @@ impl ABIMachineSpec for X64ABIMachineSpec {

next_stack = align_to(next_stack, 16);

Ok((next_stack, extra_arg))
Ok((next_stack, extra_arg_idx))
}

fn gen_load_stack(mem: StackAMode, into_reg: Writable<Reg>, ty: Type) -> Self::I {
Expand Down
Loading
Loading