Skip to content

Commit d7afdae

Browse files
authored
Unrolled build for #142389
Rollup merge of #142389 - beetrees:cranelift-arg-ext, r=bjorn3 Apply ABI attributes on return types in `rustc_codegen_cranelift` - The [x86-64 System V ABI standard](https://gitlab.com/x86-psABIs/x86-64-ABI/-/jobs/artifacts/master/raw/x86-64-ABI/abi.pdf?job=build) doesn't sign/zero-extend integer arguments or return types. - But the de-facto standard as implemented by Clang and GCC is to sign/zero-extend arguments to 32 bits (but not return types). - Additionally, Apple targets [sign/zero-extend both arguments and return values to 32 bits](https://developer.apple.com/documentation/xcode/writing-64-bit-intel-code-for-apple-platforms#Pass-arguments-to-functions-correctly). - However, the `rustc_target` ABI adjustment code currently [unconditionally extends both arguments and return values to 32 bits](https://github.com/rust-lang/rust/blame/e703dff8fe220b78195c53478e83fb2f68d8499c/compiler/rustc_target/src/callconv/x86_64.rs#L240) on all targets. - This doesn't cause a miscompilation when compiling with LLVM as LLVM will ignore the `signext`/`zeroext` attribute when applied to return types on non-Apple x86-64 targets. - Cranelift, however, does not have a similar special case, requiring `rustc` to set the argument extension attribute correctly. - However, `rustc_codegen_cranelift` doesn't currently apply ABI attributes to return types at all, meaning `rustc_codegen_cranelift` will currently miscompile `i8`/`u8`/`i16`/`u16` returns on x86-64 Apple targets as those targets require sign/zero-extension of return types. This PR fixes the bug(s) by making the `rustc_target` x86-64 System V ABI only mark return types as sign/zero-extended on Apple platforms, while also making `rustc_codegen_cranelift` apply ABI attributes to return types. The RISC-V and s390x C ABIs also require sign/zero extension of return types, so this will fix those targets when building with `rustc_codegen_cranelift` too. r? `````@bjorn3`````
2 parents f768dc0 + eb472e7 commit d7afdae

File tree

8 files changed

+1161
-27
lines changed

8 files changed

+1161
-27
lines changed

compiler/rustc_codegen_cranelift/src/abi/pass_mode.rs

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//! Argument passing
22
3-
use cranelift_codegen::ir::{ArgumentExtension, ArgumentPurpose};
3+
use cranelift_codegen::ir::ArgumentPurpose;
44
use rustc_abi::{Reg, RegKind};
55
use rustc_target::callconv::{
66
ArgAbi, ArgAttributes, ArgExtension as RustcArgExtension, CastTarget, PassMode,
@@ -32,13 +32,12 @@ fn reg_to_abi_param(reg: Reg) -> AbiParam {
3232
AbiParam::new(clif_ty)
3333
}
3434

35-
fn apply_arg_attrs_to_abi_param(mut param: AbiParam, arg_attrs: ArgAttributes) -> AbiParam {
35+
fn apply_attrs_to_abi_param(param: AbiParam, arg_attrs: ArgAttributes) -> AbiParam {
3636
match arg_attrs.arg_ext {
37-
RustcArgExtension::None => {}
38-
RustcArgExtension::Zext => param.extension = ArgumentExtension::Uext,
39-
RustcArgExtension::Sext => param.extension = ArgumentExtension::Sext,
37+
RustcArgExtension::None => param,
38+
RustcArgExtension::Zext => param.uext(),
39+
RustcArgExtension::Sext => param.sext(),
4040
}
41-
param
4241
}
4342

4443
fn cast_target_to_abi_params(cast: &CastTarget) -> SmallVec<[AbiParam; 2]> {
@@ -82,7 +81,7 @@ impl<'tcx> ArgAbiExt<'tcx> for ArgAbi<'tcx, Ty<'tcx>> {
8281
match self.mode {
8382
PassMode::Ignore => smallvec![],
8483
PassMode::Direct(attrs) => match self.layout.backend_repr {
85-
BackendRepr::Scalar(scalar) => smallvec![apply_arg_attrs_to_abi_param(
84+
BackendRepr::Scalar(scalar) => smallvec![apply_attrs_to_abi_param(
8685
AbiParam::new(scalar_to_clif_type(tcx, scalar)),
8786
attrs
8887
)],
@@ -97,8 +96,8 @@ impl<'tcx> ArgAbiExt<'tcx> for ArgAbi<'tcx, Ty<'tcx>> {
9796
let a = scalar_to_clif_type(tcx, a);
9897
let b = scalar_to_clif_type(tcx, b);
9998
smallvec![
100-
apply_arg_attrs_to_abi_param(AbiParam::new(a), attrs_a),
101-
apply_arg_attrs_to_abi_param(AbiParam::new(b), attrs_b),
99+
apply_attrs_to_abi_param(AbiParam::new(a), attrs_a),
100+
apply_attrs_to_abi_param(AbiParam::new(b), attrs_b),
102101
]
103102
}
104103
_ => unreachable!("{:?}", self.layout.backend_repr),
@@ -112,19 +111,19 @@ impl<'tcx> ArgAbiExt<'tcx> for ArgAbi<'tcx, Ty<'tcx>> {
112111
// Abi requires aligning struct size to pointer size
113112
let size = self.layout.size.align_to(tcx.data_layout.pointer_align.abi);
114113
let size = u32::try_from(size.bytes()).unwrap();
115-
smallvec![apply_arg_attrs_to_abi_param(
114+
smallvec![apply_attrs_to_abi_param(
116115
AbiParam::special(pointer_ty(tcx), ArgumentPurpose::StructArgument(size),),
117116
attrs
118117
)]
119118
} else {
120-
smallvec![apply_arg_attrs_to_abi_param(AbiParam::new(pointer_ty(tcx)), attrs)]
119+
smallvec![apply_attrs_to_abi_param(AbiParam::new(pointer_ty(tcx)), attrs)]
121120
}
122121
}
123122
PassMode::Indirect { attrs, meta_attrs: Some(meta_attrs), on_stack } => {
124123
assert!(!on_stack);
125124
smallvec![
126-
apply_arg_attrs_to_abi_param(AbiParam::new(pointer_ty(tcx)), attrs),
127-
apply_arg_attrs_to_abi_param(AbiParam::new(pointer_ty(tcx)), meta_attrs),
125+
apply_attrs_to_abi_param(AbiParam::new(pointer_ty(tcx)), attrs),
126+
apply_attrs_to_abi_param(AbiParam::new(pointer_ty(tcx)), meta_attrs),
128127
]
129128
}
130129
}
@@ -133,30 +132,46 @@ impl<'tcx> ArgAbiExt<'tcx> for ArgAbi<'tcx, Ty<'tcx>> {
133132
fn get_abi_return(&self, tcx: TyCtxt<'tcx>) -> (Option<AbiParam>, Vec<AbiParam>) {
134133
match self.mode {
135134
PassMode::Ignore => (None, vec![]),
136-
PassMode::Direct(_) => match self.layout.backend_repr {
137-
BackendRepr::Scalar(scalar) => {
138-
(None, vec![AbiParam::new(scalar_to_clif_type(tcx, scalar))])
139-
}
135+
PassMode::Direct(attrs) => match self.layout.backend_repr {
136+
BackendRepr::Scalar(scalar) => (
137+
None,
138+
vec![apply_attrs_to_abi_param(
139+
AbiParam::new(scalar_to_clif_type(tcx, scalar)),
140+
attrs,
141+
)],
142+
),
140143
BackendRepr::SimdVector { .. } => {
141144
let vector_ty = crate::intrinsics::clif_vector_type(tcx, self.layout);
142-
(None, vec![AbiParam::new(vector_ty)])
145+
(None, vec![apply_attrs_to_abi_param(AbiParam::new(vector_ty), attrs)])
143146
}
144147
_ => unreachable!("{:?}", self.layout.backend_repr),
145148
},
146-
PassMode::Pair(_, _) => match self.layout.backend_repr {
149+
PassMode::Pair(attrs_a, attrs_b) => match self.layout.backend_repr {
147150
BackendRepr::ScalarPair(a, b) => {
148151
let a = scalar_to_clif_type(tcx, a);
149152
let b = scalar_to_clif_type(tcx, b);
150-
(None, vec![AbiParam::new(a), AbiParam::new(b)])
153+
(
154+
None,
155+
vec![
156+
apply_attrs_to_abi_param(AbiParam::new(a), attrs_a),
157+
apply_attrs_to_abi_param(AbiParam::new(b), attrs_b),
158+
],
159+
)
151160
}
152161
_ => unreachable!("{:?}", self.layout.backend_repr),
153162
},
154163
PassMode::Cast { ref cast, .. } => {
155164
(None, cast_target_to_abi_params(cast).into_iter().collect())
156165
}
157-
PassMode::Indirect { attrs: _, meta_attrs: None, on_stack } => {
166+
PassMode::Indirect { attrs, meta_attrs: None, on_stack } => {
158167
assert!(!on_stack);
159-
(Some(AbiParam::special(pointer_ty(tcx), ArgumentPurpose::StructReturn)), vec![])
168+
(
169+
Some(apply_attrs_to_abi_param(
170+
AbiParam::special(pointer_ty(tcx), ArgumentPurpose::StructReturn),
171+
attrs,
172+
)),
173+
vec![],
174+
)
160175
}
161176
PassMode::Indirect { attrs: _, meta_attrs: Some(_), on_stack: _ } => {
162177
unreachable!("unsized return value")

compiler/rustc_target/src/callconv/x86_64.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use rustc_abi::{
77
};
88

99
use crate::callconv::{ArgAbi, CastTarget, FnAbi};
10+
use crate::spec::HasTargetSpec;
1011

1112
/// Classification of "eightbyte" components.
1213
// N.B., the order of the variants is from general to specific,
@@ -175,7 +176,7 @@ const MAX_SSE_REGS: usize = 8; // XMM0-7
175176
pub(crate) fn compute_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>)
176177
where
177178
Ty: TyAbiInterface<'a, C> + Copy,
178-
C: HasDataLayout,
179+
C: HasDataLayout + HasTargetSpec,
179180
{
180181
let mut int_regs = MAX_INT_REGS;
181182
let mut sse_regs = MAX_SSE_REGS;
@@ -236,7 +237,7 @@ where
236237
if arg.layout.is_aggregate() {
237238
let size = arg.layout.size;
238239
arg.cast_to(cast_target(cls, size));
239-
} else {
240+
} else if is_arg || cx.target_spec().is_like_darwin {
240241
arg.extend_integer_width_to(32);
241242
}
242243
}

tests/assembly/pic-relocation-model.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ pub fn call_other_fn() -> u8 {
1919
}
2020

2121
// CHECK-LABEL: other_fn:
22-
// CHECK: callq *foreign_fn@GOTPCREL(%rip)
22+
// CHECK: {{(jmpq|callq)}} *foreign_fn@GOTPCREL(%rip)
2323
#[no_mangle]
2424
#[inline(never)]
2525
pub fn other_fn() -> u8 {

tests/assembly/pie-relocation-model.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ pub fn call_other_fn() -> u8 {
2222
// CHECK-LABEL: other_fn:
2323
// External functions are still called through GOT, since we don't know if the symbol
2424
// is defined in the binary or in the shared library.
25-
// CHECK: callq *foreign_fn@GOTPCREL(%rip)
25+
// CHECK: {{(jmpq|callq)}} *foreign_fn@GOTPCREL(%rip)
2626
#[no_mangle]
2727
#[inline(never)]
2828
pub fn other_fn() -> u8 {

tests/codegen/pie-relocation-model.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ pub fn call_foreign_fn() -> u8 {
1313

1414
// External functions are still marked as non-dso_local, since we don't know if the symbol
1515
// is defined in the binary or in the shared library.
16-
// CHECK: declare zeroext i8 @foreign_fn()
16+
// CHECK: declare i8 @foreign_fn()
1717
extern "C" {
1818
fn foreign_fn() -> u8;
1919
}

0 commit comments

Comments
 (0)