Bad codegen for array::map-based code

[newpavlov]

array::map results in a bad codegen compared to manually unrolled code. This issue can be demonstrated by the following code: https://rust.godbolt.org/z/T7bnTE398

Relevant issue: #86912

[newpavlov]

A more straightforward implementation of array::map makes codegen identical: https://rust.godbolt.org/z/a9csPcnob But it's unclear how to properly make it panic-safe in regards to f, i.e. we need a way to properly drop created Us after f has panicked.

[the8472]

We could special-case based on needs_drop::<U>?

[veber-alex]

There is another issue here, the simple map function produces very bad assembly on nightly.

[newpavlov]

@the8472
No, I think we simply need something like ReadBuf to track number of initialized elements. With no-drop types the additional code should get eliminated.

@veber-alex
It looks like a clear nightly regression. I will open an issue for it.

[the8472]

I think we simply need something like ReadBuf to track number of initialized elements

The Guard type does just that.
The thing is that the surrounding code is super-generic and relies heavily on the optimizer. That it runs on two loop conditions (0..N and iter.next()) probably doesn't help, LLVM is bad at optimizing that kind of thing.

[newpavlov]

I wrote variation of the Guard and it properly compiles into expected assembly. Dunno what exactly makes the difference.

[Rageking8]

@rustbot label +A-codegen +C-bug

[InBetweenNames]

What would it take to get @newpavlov's variation upstreamed? When dealing with vectorized code, it would be really nice to not have to unroll everything manually and use just use map.

[memoryruins]

Testing with rustc 1.67.0-nightly (e1d819583 2022-12-05), the initial example using <[T; N]>::map in foo2 produces the same codegen as the manually unrolled code. Whichever change improved its codegen has not reached beta yet.

[HadrienG2]

Another performance pitfall of array::map is that it isn't marked #[inline], and sometimes (think codegen-units != 1), the compiler will take this opportunity to fail to inline it, resulting in disaster when the mapping is very simple and at the bottom of a hot loop.

This wouldn't be so bad if it only concerned array::map, which performance-conscious people should use sparingly anyway (overly enthusiastic use tends to create lots of data movement to and from the stack, which LLVM can have a hard time optimizing). But the problem as far as I am concerned is that array::map is also currently a building block of array::from_fn, which is pretty much the standard way to safely initialize arrays of inhomogeneous values and unknown length in current Rust. And not being able to use that in performance-sensitive code is a much greater loss.

I think either array::map should be made #[inline], or the implementation of array::from_fn should be changed to refrain from relying on array::map.