From 00bd24766ff31f4234c72677f978e198ff8cac82 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Sat, 6 Apr 2024 11:37:57 -0700 Subject: Don't emit divide-by-zero panic paths in `StepBy::len` I happened to notice today that there's actually two such calls emitted in the assembly: Since they're impossible, hopefully telling LLVM that will also help optimizations elsewhere. --- tests/codegen/step_by-overflow-checks.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 tests/codegen/step_by-overflow-checks.rs (limited to 'tests/codegen') diff --git a/tests/codegen/step_by-overflow-checks.rs b/tests/codegen/step_by-overflow-checks.rs new file mode 100644 index 00000000000..43e8514a8b7 --- /dev/null +++ b/tests/codegen/step_by-overflow-checks.rs @@ -0,0 +1,26 @@ +//@ compile-flags: -O + +#![crate_type = "lib"] + +use std::iter::StepBy; +use std::slice::Iter; + +// The constructor for `StepBy` ensures we can never end up needing to do zero +// checks on denominators, so check that the code isn't emitting panic paths. + +// CHECK-LABEL: @step_by_len_std +#[no_mangle] +pub fn step_by_len_std(x: &StepBy>) -> usize { + // CHECK-NOT: div_by_zero + // CHECK: udiv + // CHECK-NOT: div_by_zero + x.len() +} + +// CHECK-LABEL: @step_by_len_naive +#[no_mangle] +pub fn step_by_len_naive(x: Iter, step_minus_one: usize) -> usize { + // CHECK: udiv + // CHECK: call{{.+}}div_by_zero + x.len() / (step_minus_one + 1) +} -- cgit 1.4.1-3-g733a5 From 009280c5e312bdf11cd0e0e1b336bf374eed7b00 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Wed, 20 Mar 2024 14:38:32 +0100 Subject: Fix argument ABI for overaligned structs on ppc64le When passing a 16 (or higher) aligned struct by value on ppc64le, it needs to be passed as an array of `i128` rather than an array of `i64`. This will force the use of an even starting register. For the case of a 16 byte struct with alignment 16 it is important that `[1 x i128]` is used instead of `i128` -- apparently, the latter will get treated similarly to `[2 x i64]`, not exhibiting the correct ABI. Add a `force_array` flag to `Uniform` to support this. The relevant clang code can be found here: https://github.com/llvm/llvm-project/blob/fe2119a7b08b6e468b2a67768904ea85b1bf0a45/clang/lib/CodeGen/Targets/PPC.cpp#L878-L884 https://github.com/llvm/llvm-project/blob/fe2119a7b08b6e468b2a67768904ea85b1bf0a45/clang/lib/CodeGen/Targets/PPC.cpp#L780-L784 I think the corresponding psABI wording is this: > Fixed size aggregates and unions passed by value are mapped to as > many doublewords of the parameter save area as the value uses in > memory. Aggregrates and unions are aligned according to their > alignment requirements. This may result in doublewords being > skipped for alignment. In particular the last sentence. Fixes https://github.com/rust-lang/rust/issues/122767. --- compiler/rustc_codegen_llvm/src/abi.rs | 2 +- compiler/rustc_target/src/abi/call/aarch64.rs | 8 +-- compiler/rustc_target/src/abi/call/arm.rs | 10 ++- compiler/rustc_target/src/abi/call/csky.rs | 4 +- compiler/rustc_target/src/abi/call/loongarch.rs | 7 +- compiler/rustc_target/src/abi/call/mips.rs | 5 +- compiler/rustc_target/src/abi/call/mips64.rs | 4 +- compiler/rustc_target/src/abi/call/mod.rs | 5 +- compiler/rustc_target/src/abi/call/nvptx64.rs | 2 +- compiler/rustc_target/src/abi/call/powerpc64.rs | 25 ++++--- compiler/rustc_target/src/abi/call/riscv.rs | 7 +- compiler/rustc_target/src/abi/call/sparc.rs | 5 +- compiler/rustc_target/src/abi/call/sparc64.rs | 4 +- compiler/rustc_target/src/abi/call/wasm.rs | 4 +- tests/codegen/powerpc64le-struct-align-128.rs | 93 +++++++++++++++++++++++++ 15 files changed, 152 insertions(+), 33 deletions(-) create mode 100644 tests/codegen/powerpc64le-struct-align-128.rs (limited to 'tests/codegen') diff --git a/compiler/rustc_codegen_llvm/src/abi.rs b/compiler/rustc_codegen_llvm/src/abi.rs index d2828669d43..8b2330471f7 100644 --- a/compiler/rustc_codegen_llvm/src/abi.rs +++ b/compiler/rustc_codegen_llvm/src/abi.rs @@ -150,7 +150,7 @@ impl LlvmType for CastTarget { // Simplify to a single unit or an array if there's no prefix. // This produces the same layout, but using a simpler type. if self.prefix.iter().all(|x| x.is_none()) { - if rest_count == 1 { + if rest_count == 1 && !self.rest.force_array { return rest_ll_unit; } diff --git a/compiler/rustc_target/src/abi/call/aarch64.rs b/compiler/rustc_target/src/abi/call/aarch64.rs index f99f6a3b721..bf9cf232d81 100644 --- a/compiler/rustc_target/src/abi/call/aarch64.rs +++ b/compiler/rustc_target/src/abi/call/aarch64.rs @@ -31,7 +31,7 @@ where RegKind::Vector => size.bits() == 64 || size.bits() == 128, }; - valid_unit.then_some(Uniform { unit, total: size }) + valid_unit.then_some(Uniform { unit, total: size, force_array: false }) }) } @@ -60,7 +60,7 @@ where let size = ret.layout.size; let bits = size.bits(); if bits <= 128 { - ret.cast_to(Uniform { unit: Reg::i64(), total: size }); + ret.cast_to(Uniform { unit: Reg::i64(), total: size, force_array: false }); return; } ret.make_indirect(); @@ -100,9 +100,9 @@ where }; if size.bits() <= 128 { if align.bits() == 128 { - arg.cast_to(Uniform { unit: Reg::i128(), total: size }); + arg.cast_to(Uniform { unit: Reg::i128(), total: size, force_array: false }); } else { - arg.cast_to(Uniform { unit: Reg::i64(), total: size }); + arg.cast_to(Uniform { unit: Reg::i64(), total: size, force_array: false }); } return; } diff --git a/compiler/rustc_target/src/abi/call/arm.rs b/compiler/rustc_target/src/abi/call/arm.rs index 95f6691d42a..177a16bafc8 100644 --- a/compiler/rustc_target/src/abi/call/arm.rs +++ b/compiler/rustc_target/src/abi/call/arm.rs @@ -21,7 +21,7 @@ where RegKind::Vector => size.bits() == 64 || size.bits() == 128, }; - valid_unit.then_some(Uniform { unit, total: size }) + valid_unit.then_some(Uniform { unit, total: size, force_array: false }) }) } @@ -49,7 +49,7 @@ where let size = ret.layout.size; let bits = size.bits(); if bits <= 32 { - ret.cast_to(Uniform { unit: Reg::i32(), total: size }); + ret.cast_to(Uniform { unit: Reg::i32(), total: size, force_array: false }); return; } ret.make_indirect(); @@ -78,7 +78,11 @@ where let align = arg.layout.align.abi.bytes(); let total = arg.layout.size; - arg.cast_to(Uniform { unit: if align <= 4 { Reg::i32() } else { Reg::i64() }, total }); + arg.cast_to(Uniform { + unit: if align <= 4 { Reg::i32() } else { Reg::i64() }, + total, + force_array: false, + }); } pub fn compute_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>) diff --git a/compiler/rustc_target/src/abi/call/csky.rs b/compiler/rustc_target/src/abi/call/csky.rs index 8b4328db52e..dae428e3538 100644 --- a/compiler/rustc_target/src/abi/call/csky.rs +++ b/compiler/rustc_target/src/abi/call/csky.rs @@ -18,7 +18,7 @@ fn classify_ret(arg: &mut ArgAbi<'_, Ty>) { if total.bits() > 64 { arg.make_indirect(); } else if total.bits() > 32 { - arg.cast_to(Uniform { unit: Reg::i32(), total }); + arg.cast_to(Uniform { unit: Reg::i32(), total, force_array: false }); } else { arg.cast_to(Reg::i32()); } @@ -38,7 +38,7 @@ fn classify_arg(arg: &mut ArgAbi<'_, Ty>) { if arg.layout.is_aggregate() { let total = arg.layout.size; if total.bits() > 32 { - arg.cast_to(Uniform { unit: Reg::i32(), total }); + arg.cast_to(Uniform { unit: Reg::i32(), total, force_array: false }); } else { arg.cast_to(Reg::i32()); } diff --git a/compiler/rustc_target/src/abi/call/loongarch.rs b/compiler/rustc_target/src/abi/call/loongarch.rs index 35d4b331cb4..3b8a3ba66ba 100644 --- a/compiler/rustc_target/src/abi/call/loongarch.rs +++ b/compiler/rustc_target/src/abi/call/loongarch.rs @@ -195,7 +195,11 @@ where if total.bits() <= xlen { arg.cast_to(xlen_reg); } else { - arg.cast_to(Uniform { unit: xlen_reg, total: Size::from_bits(xlen * 2) }); + arg.cast_to(Uniform { + unit: xlen_reg, + total: Size::from_bits(xlen * 2), + force_array: false, + }); } return false; } @@ -281,6 +285,7 @@ fn classify_arg<'a, Ty, C>( arg.cast_to(Uniform { unit: if align_regs { double_xlen_reg } else { xlen_reg }, total: Size::from_bits(xlen * 2), + force_array: false, }); } if align_regs && is_vararg { diff --git a/compiler/rustc_target/src/abi/call/mips.rs b/compiler/rustc_target/src/abi/call/mips.rs index 57ccfe2152b..054c127181a 100644 --- a/compiler/rustc_target/src/abi/call/mips.rs +++ b/compiler/rustc_target/src/abi/call/mips.rs @@ -27,7 +27,10 @@ where if arg.layout.is_aggregate() { let pad_i32 = !offset.is_aligned(align); - arg.cast_to_and_pad_i32(Uniform { unit: Reg::i32(), total: size }, pad_i32); + arg.cast_to_and_pad_i32( + Uniform { unit: Reg::i32(), total: size, force_array: false }, + pad_i32, + ); } else { arg.extend_integer_width_to(32); } diff --git a/compiler/rustc_target/src/abi/call/mips64.rs b/compiler/rustc_target/src/abi/call/mips64.rs index 2700f67b209..69e73beca9a 100644 --- a/compiler/rustc_target/src/abi/call/mips64.rs +++ b/compiler/rustc_target/src/abi/call/mips64.rs @@ -68,7 +68,7 @@ where } // Cast to a uniform int structure - ret.cast_to(Uniform { unit: Reg::i64(), total: size }); + ret.cast_to(Uniform { unit: Reg::i64(), total: size, force_array: false }); } else { ret.make_indirect(); } @@ -139,7 +139,7 @@ where let rest_size = size - Size::from_bytes(8) * prefix_index as u64; arg.cast_to(CastTarget { prefix, - rest: Uniform { unit: Reg::i64(), total: rest_size }, + rest: Uniform { unit: Reg::i64(), total: rest_size, force_array: false }, attrs: ArgAttributes { regular: ArgAttribute::default(), arg_ext: ArgExtension::None, diff --git a/compiler/rustc_target/src/abi/call/mod.rs b/compiler/rustc_target/src/abi/call/mod.rs index cdd3f0afd79..022ea44cc93 100644 --- a/compiler/rustc_target/src/abi/call/mod.rs +++ b/compiler/rustc_target/src/abi/call/mod.rs @@ -255,11 +255,14 @@ pub struct Uniform { /// for 64-bit integers with a total size of 20 bytes. When the argument is actually passed, /// this size will be rounded up to the nearest multiple of `unit.size`. pub total: Size, + + /// Force the use of an array, even if there is only a single element. + pub force_array: bool, } impl From for Uniform { fn from(unit: Reg) -> Uniform { - Uniform { unit, total: unit.size } + Uniform { unit, total: unit.size, force_array: false } } } diff --git a/compiler/rustc_target/src/abi/call/nvptx64.rs b/compiler/rustc_target/src/abi/call/nvptx64.rs index 5c040ce9c3b..e7525c76015 100644 --- a/compiler/rustc_target/src/abi/call/nvptx64.rs +++ b/compiler/rustc_target/src/abi/call/nvptx64.rs @@ -35,7 +35,7 @@ where 16 => Reg::i128(), _ => unreachable!("Align is given as power of 2 no larger than 16 bytes"), }; - arg.cast_to(Uniform { unit, total: Size::from_bytes(2 * align_bytes) }); + arg.cast_to(Uniform { unit, total: Size::from_bytes(2 * align_bytes), force_array: false }); } else { // FIXME: find a better way to do this. See https://github.com/rust-lang/rust/issues/117271. arg.make_direct_deprecated(); diff --git a/compiler/rustc_target/src/abi/call/powerpc64.rs b/compiler/rustc_target/src/abi/call/powerpc64.rs index 2d41f77e50e..0b2bd8c90d7 100644 --- a/compiler/rustc_target/src/abi/call/powerpc64.rs +++ b/compiler/rustc_target/src/abi/call/powerpc64.rs @@ -2,7 +2,7 @@ // Alignment of 128 bit types is not currently handled, this will // need to be fixed when PowerPC vector support is added. -use crate::abi::call::{ArgAbi, FnAbi, Reg, RegKind, Uniform}; +use crate::abi::call::{Align, ArgAbi, FnAbi, Reg, RegKind, Uniform}; use crate::abi::{Endian, HasDataLayout, TyAbiInterface}; use crate::spec::HasTargetSpec; @@ -37,7 +37,7 @@ where RegKind::Vector => arg.layout.size.bits() == 128, }; - valid_unit.then_some(Uniform { unit, total: arg.layout.size }) + valid_unit.then_some(Uniform { unit, total: arg.layout.size, force_array: false }) }) } @@ -81,7 +81,7 @@ where Reg::i64() }; - ret.cast_to(Uniform { unit, total: size }); + ret.cast_to(Uniform { unit, total: size, force_array: false }); return; } @@ -108,18 +108,21 @@ where } let size = arg.layout.size; - let (unit, total) = if size.bits() <= 64 { + if size.bits() <= 64 { // Aggregates smaller than a doubleword should appear in // the least-significant bits of the parameter doubleword. - (Reg { kind: RegKind::Integer, size }, size) + arg.cast_to(Reg { kind: RegKind::Integer, size }) } else { - // Aggregates larger than a doubleword should be padded - // at the tail to fill out a whole number of doublewords. - let reg_i64 = Reg::i64(); - (reg_i64, size.align_to(reg_i64.align(cx))) + // Aggregates larger than i64 should be padded at the tail to fill out a whole number + // of i64s or i128s, depending on the aggregate alignment. Always use an array for + // this, even if there is only a single element. + let reg = if arg.layout.align.abi.bytes() > 8 { Reg::i128() } else { Reg::i64() }; + arg.cast_to(Uniform { + unit: reg, + total: size.align_to(Align::from_bytes(reg.size.bytes()).unwrap()), + force_array: true, + }) }; - - arg.cast_to(Uniform { unit, total }); } pub fn compute_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>) diff --git a/compiler/rustc_target/src/abi/call/riscv.rs b/compiler/rustc_target/src/abi/call/riscv.rs index 6a38496dc57..0dad35c8aab 100644 --- a/compiler/rustc_target/src/abi/call/riscv.rs +++ b/compiler/rustc_target/src/abi/call/riscv.rs @@ -201,7 +201,11 @@ where if total.bits() <= xlen { arg.cast_to(xlen_reg); } else { - arg.cast_to(Uniform { unit: xlen_reg, total: Size::from_bits(xlen * 2) }); + arg.cast_to(Uniform { + unit: xlen_reg, + total: Size::from_bits(xlen * 2), + force_array: false, + }); } return false; } @@ -287,6 +291,7 @@ fn classify_arg<'a, Ty, C>( arg.cast_to(Uniform { unit: if align_regs { double_xlen_reg } else { xlen_reg }, total: Size::from_bits(xlen * 2), + force_array: false, }); } if align_regs && is_vararg { diff --git a/compiler/rustc_target/src/abi/call/sparc.rs b/compiler/rustc_target/src/abi/call/sparc.rs index 57ccfe2152b..054c127181a 100644 --- a/compiler/rustc_target/src/abi/call/sparc.rs +++ b/compiler/rustc_target/src/abi/call/sparc.rs @@ -27,7 +27,10 @@ where if arg.layout.is_aggregate() { let pad_i32 = !offset.is_aligned(align); - arg.cast_to_and_pad_i32(Uniform { unit: Reg::i32(), total: size }, pad_i32); + arg.cast_to_and_pad_i32( + Uniform { unit: Reg::i32(), total: size, force_array: false }, + pad_i32, + ); } else { arg.extend_integer_width_to(32); } diff --git a/compiler/rustc_target/src/abi/call/sparc64.rs b/compiler/rustc_target/src/abi/call/sparc64.rs index cbed5b4afc1..a36b691468e 100644 --- a/compiler/rustc_target/src/abi/call/sparc64.rs +++ b/compiler/rustc_target/src/abi/call/sparc64.rs @@ -192,7 +192,7 @@ where arg.cast_to(CastTarget { prefix: data.prefix, - rest: Uniform { unit: Reg::i64(), total: rest_size }, + rest: Uniform { unit: Reg::i64(), total: rest_size, force_array: false }, attrs: ArgAttributes { regular: data.arg_attribute, arg_ext: ArgExtension::None, @@ -205,7 +205,7 @@ where } } - arg.cast_to(Uniform { unit: Reg::i64(), total }); + arg.cast_to(Uniform { unit: Reg::i64(), total, force_array: false }); } pub fn compute_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>) diff --git a/compiler/rustc_target/src/abi/call/wasm.rs b/compiler/rustc_target/src/abi/call/wasm.rs index a7a2b314a94..a773fb1e814 100644 --- a/compiler/rustc_target/src/abi/call/wasm.rs +++ b/compiler/rustc_target/src/abi/call/wasm.rs @@ -1,4 +1,4 @@ -use crate::abi::call::{ArgAbi, FnAbi, Uniform}; +use crate::abi::call::{ArgAbi, FnAbi}; use crate::abi::{HasDataLayout, TyAbiInterface}; fn unwrap_trivial_aggregate<'a, Ty, C>(cx: &C, val: &mut ArgAbi<'a, Ty>) -> bool @@ -10,7 +10,7 @@ where if let Some(unit) = val.layout.homogeneous_aggregate(cx).ok().and_then(|ha| ha.unit()) { let size = val.layout.size; if unit.size == size { - val.cast_to(Uniform { unit, total: size }); + val.cast_to(unit); return true; } } diff --git a/tests/codegen/powerpc64le-struct-align-128.rs b/tests/codegen/powerpc64le-struct-align-128.rs new file mode 100644 index 00000000000..0096c6d3138 --- /dev/null +++ b/tests/codegen/powerpc64le-struct-align-128.rs @@ -0,0 +1,93 @@ +// Test that structs aligned to 128 bits are passed with the correct ABI on powerpc64le. +// This is similar to aarch64-struct-align-128.rs, but for ppc. + +//@ compile-flags: --target powerpc64le-unknown-linux-gnu +//@ needs-llvm-components: powerpc + +#![feature(no_core, lang_items)] +#![crate_type = "lib"] +#![no_core] + +#[lang="sized"] +trait Sized { } +#[lang="freeze"] +trait Freeze { } +#[lang="copy"] +trait Copy { } + +#[repr(C)] +pub struct Align8 { + pub a: u64, + pub b: u64, +} + +#[repr(transparent)] +pub struct Transparent8 { + a: Align8 +} + +#[repr(C)] +pub struct Wrapped8 { + a: Align8, +} + +extern "C" { + // CHECK: declare void @test_8([2 x i64], [2 x i64], [2 x i64]) + fn test_8(a: Align8, b: Transparent8, c: Wrapped8); +} + +#[repr(C)] +#[repr(align(16))] +pub struct Align16 { + pub a: u64, + pub b: u64, +} + +#[repr(transparent)] +pub struct Transparent16 { + a: Align16 +} + +#[repr(C)] +pub struct Wrapped16 { + pub a: Align16, +} + +extern "C" { + // It's important that this produces [1 x i128] rather than just i128! + // CHECK: declare void @test_16([1 x i128], [1 x i128], [1 x i128]) + fn test_16(a: Align16, b: Transparent16, c: Wrapped16); +} + +#[repr(C)] +#[repr(align(32))] +pub struct Align32 { + pub a: u64, + pub b: u64, + pub c: u64, +} + +#[repr(transparent)] +pub struct Transparent32 { + a: Align32 +} + +#[repr(C)] +pub struct Wrapped32 { + pub a: Align32, +} + +extern "C" { + // CHECK: declare void @test_32([2 x i128], [2 x i128], [2 x i128]) + fn test_32(a: Align32, b: Transparent32, c: Wrapped32); +} + +pub unsafe fn main( + a1: Align8, a2: Transparent8, a3: Wrapped8, + b1: Align16, b2: Transparent16, b3: Wrapped16, + c1: Align32, c2: Transparent32, c3: Wrapped32, +) { + test_8(a1, a2, a3); + test_16(b1, b2, b3); + test_32(c1, c2, c3); +} -- cgit 1.4.1-3-g733a5