unit and [1 x unit] layouts are incompatible on big-endian targets

[folkertdev]

cc @beetrees @RalfJung

Observe

https://godbolt.org/z/G6qrdT1Ez

target triple = "aarch64_be"

define float @unit([8 x float] %claim_all_regs, float %x) {
  ret float %x
}

define float @array([8 x float] %claim_all_regs, [1 x float] %x) {
  %v = extractvalue [1 x float] %x, 0
  ret float %v
}

results in

unit:                                   // @unit
        ldr     s0, [sp, #4]
        ret
array:                                  // @array
        ldr     s0, [sp]
        ret

So the value is stored in different halves of the register-sized stack slot when it is unit versus [1 x unit].

---

On powerpc64 (also a big-endian target) the layouts are also not equivalent:

https://godbolt.org/z/szfr6xPM7

in this case we want the bare float semantics

https://godbolt.org/z/vGKczqzcE

No other big-endian targets use Uniform::consecutive from what I can tell.

---

Maybe there is a better way to fix this? Like guarding on endianness if we really want the simpler LLVM type on LE.

[RalfJung]

LLVM ABI continues to be a treasure trove...

@nikic is this expected behavior?

[nikic]

I think so? LLVM doesn't seem to be doing anything particularly interesting here. If the psABI says that a float and a 1-element float HFA have different ABI, then... that's just how it is.

https://clang.godbolt.org/z/TMec497aj just to double check that Clang and GCC agree on the ABI here.

[folkertdev]

@bors try jobs=test-various

r? beetrees (or @nikic if you want)

[rustbot]

beetrees is currently at their maximum review capacity.
They may take a while to respond.

[rust-bors]

☀️ Try build successful (CI)
Build commit: 38be567 (38be5679e8f8fcf6f4df3323a741e42f6f394551)
Base parent: 5ceaf66 (5ceaf6608eb354c2f5bbb3b8d974caa367dac81c)

[beetrees]

Could you also add an assembly test for PowerPC64 and AArch64 that would fail if the wrong one of Uniform::new/Uniform::consecutive was used?

View changes since this review

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[folkertdev]

@rustbot ready

Added an llvm IR test (just like the others), assuming that is what you meant?

[beetrees]

I was thinking an assembly LLVM test that would fail if unit was used instead of [1 x unit] (or vice versa). Something like

#[repr(C)]
struct Hfa(f32);

#[unsafe(no_mangle)]
extern "C" fn float_hfa(_: f32, _: f32, _: f32, _: f32, _: f32, _: f32, _: f32, _: f32, x: Hfa) -> f32 {
    // CHECK-LABEL: float_hfa
    // CHECK: ldr s0, [sp]
    x.0
}

for AArch64 BE and the equivalent for PowerPC64.

@rustbot author

[folkertdev]

Here you go

@rustbot ready

View changes since the review

[beetrees]

Thanks.

@bors r+ rollup

[rust-bors]

📌 Commit b1a3475 has been approved by beetrees

It is now in the queue for this repository.

[rust-timer]

Note

This PR was benchmarked as part of triage of its containing rollup: triage URL.

Finished benchmarking commit (90495a8): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.2%	[-0.2%, -0.2%]	1
Improvements ✅ (secondary)	-0.3%	[-0.5%, -0.1%]	11
All ❌✅ (primary)	-0.2%	[-0.2%, -0.2%]	1

Max RSS (memory usage)

This perf run didn't have relevant results for this metric.

Cycles

This perf run didn't have relevant results for this metric.

Binary size

This perf run didn't have relevant results for this metric.

Artifact size: 408.59 MiB -> 408.68 MiB (0.02%)

[JonathanBrouwer]

This is noise, all PRs in the rollup have the same results