Skip to content

Commit

Permalink
Rollup merge of #112096 - workingjubilee:array-unzip, r=scottmcm
Browse files Browse the repository at this point in the history
Remove array_zip

`[T; N]::zip` is "eager" but most zips are mapped. This causes poor optimization in generated code. This is a fundamental design issue and "zip" is "prime real estate" in terms of function names, so let's free it up again.

- FCP concluded in #80094 (comment)
- Closes #80094
- Closes #103555

Could use review to make sure we aren't losing any essential codegen tests.
r? `@scottmcm`
  • Loading branch information
matthiaskrgr committed May 31, 2023
2 parents 183a31b + 374f5a8 commit 88160ab
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 38 deletions.
23 changes: 0 additions & 23 deletions library/core/src/array/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -538,29 +538,6 @@ impl<T, const N: usize> [T; N] {
drain_array_with(self, |iter| try_from_trusted_iterator(iter.map(f)))
}

/// 'Zips up' two arrays into a single array of pairs.
///
/// `zip()` returns a new array where every element is a tuple where the
/// first element comes from the first array, and the second element comes
/// from the second array. In other words, it zips two arrays together,
/// into a single one.
///
/// # Examples
///
/// ```
/// #![feature(array_zip)]
/// let x = [1, 2, 3];
/// let y = [4, 5, 6];
/// let z = x.zip(y);
/// assert_eq!(z, [(1, 4), (2, 5), (3, 6)]);
/// ```
#[unstable(feature = "array_zip", issue = "80094")]
pub fn zip<U>(self, rhs: [U; N]) -> [(T, U); N] {
drain_array_with(self, |lhs| {
drain_array_with(rhs, |rhs| from_trusted_iterator(crate::iter::zip(lhs, rhs)))
})
}

/// Returns a slice containing the entire array. Equivalent to `&s[..]`.
#[stable(feature = "array_as_slice", since = "1.57.0")]
#[rustc_const_stable(feature = "array_as_slice", since = "1.57.0")]
Expand Down
11 changes: 0 additions & 11 deletions tests/codegen/array-map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
// ignore-debug (the extra assertions get in the way)

#![crate_type = "lib"]
#![feature(array_zip)]

// CHECK-LABEL: @short_integer_map
#[no_mangle]
Expand All @@ -16,16 +15,6 @@ pub fn short_integer_map(x: [u32; 8]) -> [u32; 8] {
x.map(|x| 2 * x + 1)
}

// CHECK-LABEL: @short_integer_zip_map
#[no_mangle]
pub fn short_integer_zip_map(x: [u32; 8], y: [u32; 8]) -> [u32; 8] {
// CHECK: %[[A:.+]] = load <8 x i32>
// CHECK: %[[B:.+]] = load <8 x i32>
// CHECK: sub <8 x i32> %[[B]], %[[A]]
// CHECK: store <8 x i32>
x.zip(y).map(|(x, y)| x - y)
}

// This test is checking that LLVM can SRoA away a bunch of the overhead,
// like fully moving the iterators to registers. Notably, previous implementations
// of `map` ended up `alloca`ing the whole `array::IntoIterator`, meaning both a
Expand Down
7 changes: 3 additions & 4 deletions tests/codegen/autovectorize-f32x4.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// compile-flags: -C opt-level=3 -Z merge-functions=disabled
// only-x86_64
#![crate_type = "lib"]
#![feature(array_zip)]

// CHECK-LABEL: @auto_vectorize_direct
#[no_mangle]
Expand Down Expand Up @@ -32,12 +31,12 @@ pub fn auto_vectorize_loop(a: [f32; 4], b: [f32; 4]) -> [f32; 4] {
c
}

// CHECK-LABEL: @auto_vectorize_array_zip_map
// CHECK-LABEL: @auto_vectorize_array_from_fn
#[no_mangle]
pub fn auto_vectorize_array_zip_map(a: [f32; 4], b: [f32; 4]) -> [f32; 4] {
pub fn auto_vectorize_array_from_fn(a: [f32; 4], b: [f32; 4]) -> [f32; 4] {
// CHECK: load <4 x float>
// CHECK: load <4 x float>
// CHECK: fadd <4 x float>
// CHECK: store <4 x float>
a.zip(b).map(|(a, b)| a + b)
std::array::from_fn(|i| a[i] + b[i])
}

0 comments on commit 88160ab

Please sign in to comment.