Skip to content

fix is_homogeneous_aggregate: use unit size, not total size - #161394

Open
folkertdev wants to merge 1 commit into
rust-lang:mainfrom
folkertdev:arm-aarch64-hva-vector-unit
Open

fix is_homogeneous_aggregate: use unit size, not total size #161394
folkertdev wants to merge 1 commit into
rust-lang:mainfrom
folkertdev:arm-aarch64-hva-vector-unit

Conversation

@folkertdev

Copy link
Copy Markdown
Contributor

fixes #161382

The vector case of the three (arm, aarch64, powerpc64) is_homogeneous_aggregate implementations looked at the total type size, instead of the unit size.

blocked on #161070 merging

r? davidtwco

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 20, 2026
@folkertdev folkertdev changed the title Arm aarch64 hva vector unit fix is_homogeneous_aggregate: use unit size, not total size Aug 20, 2026
// do not get homogeneous aggregate treatment.
RegKind::Float => cx.target_spec().rustc_abi != Some(RustcAbi::Softfloat),
RegKind::Vector { .. } => size.bits() == 64 || size.bits() == 128,
RegKind::Vector { .. } => unit.size.bits() == 64 || unit.size.bits() == 128,

@folkertdev folkertdev Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines -22 to +29
RegKind::Vector { .. } => size.bits() == 64 || size.bits() == 128,
RegKind::Vector { .. } => unit.size.bits() == 64 || unit.size.bits() == 128,

@folkertdev folkertdev Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RegKind::Integer => false,
RegKind::Float => true,
RegKind::Vector { .. } => arg.layout.size.bits() == 128,
RegKind::Vector { .. } => unit.size.bits() == 128,

@folkertdev folkertdev Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nazar-pc nazar-pc Aug 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth adding u64 aggregates here too?

View changes since the review

@rust-log-analyzer

This comment has been minimized.

@folkertdev
folkertdev force-pushed the arm-aarch64-hva-vector-unit branch from a91d2f2 to fd3fd89 Compare August 20, 2026 15:31
Comment on lines +55 to +58
// When the fields are not 64 or 128 bits in size, they do not qualify as a homogeneous
// aggregate, and passed as type-erased sequences of integers.

// CHECK: define [4 x <4 x i8>] @quad_int8x4_t([4 x <4 x i8>] {{.*}} %0)
// CHECK: define [2 x i64] @quad_int8x4_t([2 x i64] {{.*}} %0)

@folkertdev folkertdev Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these previously passed the aggregate check, but that was wrong

View changes since the review

Comment on lines -77 to +80
// Other configurations are not passed by-value but indirectly.
// Other configurations passed directly when they qualify as a homogeneous aggregate.

// CHECK: define void @pair_int128x1_t
// CHECK: define [2 x <1 x i128>] @pair_int128x1_t([2 x <1 x i128>]

@folkertdev folkertdev Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these are now recognized as homogeneous aggregates

View changes since the review

@folkertdev
folkertdev force-pushed the arm-aarch64-hva-vector-unit branch from fd3fd89 to 8a59847 Compare August 20, 2026 18:14
@rustbot rustbot added the A-test-infra-minicore Area: `minicore` test auxiliary and `//@ add-core-stubs` label Aug 20, 2026
@rustbot

This comment has been minimized.

@davidtwco

Copy link
Copy Markdown
Member

@bors r+ rollup

@rust-bors

rust-bors Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 8a59847 has been approved by davidtwco

It is now in the queue for this repository.

@rust-bors rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 4, 2026
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 4, 2026
…unit, r=davidtwco

fix `is_homogeneous_aggregate`: use unit size, not total size

fixes rust-lang#161382

The vector case of the three (arm, aarch64, powerpc64) `is_homogeneous_aggregate` implementations looked at the total type size, instead of the unit size.

blocked on rust-lang#161070 merging

r? davidtwco
@GuillaumeGomez

Copy link
Copy Markdown
Member

Failed in #162297. Might be a conflict between two PRs.

@bors r-

@rust-bors rust-bors Bot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 4, 2026
@rust-bors

rust-bors Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

This pull request was unapproved.

View changes since this unapproval

@folkertdev
folkertdev force-pushed the arm-aarch64-hva-vector-unit branch from 8a59847 to a305a9c Compare September 4, 2026 14:15
@rustbot

rustbot commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

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

Copy link
Copy Markdown
Contributor Author

Yup simple merge conflict

@bors r=davidtwco

@rust-bors

rust-bors Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

📌 Commit a305a9c has been approved by davidtwco

It is now in the queue for this repository.

@rust-bors rust-bors Bot added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Sep 4, 2026
@rust-bors rust-bors Bot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 4, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Sep 4, 2026
…unit, r=davidtwco

fix `is_homogeneous_aggregate`: use unit size, not total size

fixes rust-lang#161382

The vector case of the three (arm, aarch64, powerpc64) `is_homogeneous_aggregate` implementations looked at the total type size, instead of the unit size.

blocked on rust-lang#161070 merging

r? davidtwco
rust-bors Bot pushed a commit that referenced this pull request Sep 5, 2026
…uwer

Rollup of 25 pull requests

Successful merges:

 - #162072 (Add new Tier-3 target: `powerpc64-sony-ps3`)
 - #162179 (type system const items via direct rhs)
 - #162255 (let people change rustc-dev-guide in-tree)
 - #162262 (Avoid manually instantiating some binders in error reporting with `-Znext-solver`)
 - #162277 (Introduce `rustc_middle::middel::resolve`)
 - #162285 (box: fixup map/try_map deallocate calls)
 - #162286 (string: don't unwind prematurely)
 - #162289 (alloc: a bunch of safety comments)
 - #162290 (abby test DSL: AliasTyOutlivesViaEnv)
 - #162292 (Update `askama` version to `0.16.1`)
 - #160509 (Remove `RegionExt`; move methods to `Region` in `rustc_type_ir`)
 - #160906 (Suggest usize instead of placeholder type for array length constants)
 - #160936 (traits: Represent live alias arguments as bitsets)
 - #161394 (fix `is_homogeneous_aggregate`: use unit size, not total size )
 - #161400 (Improve diagnostics for references to closures)
 - #161656 (Suggest mutable references for FnMut closure arguments)
 - #161711 (Add more splat fn type tests)
 - #161786 (Make `tcx.def_id_partial_cmp` public)
 - #161953 (sanitizers: Implicitly disable mutually exclusive sanitizers)
 - #162155 (add suggestion for `rustc_allowed_through_unstable_modules` attribute)
 - #162212 (Implement `Rng` for `Box`)
 - #162246 (Fix incorrect meta span)
 - #162266 (std: fix typo)
 - #162291 (Add regression test from 1.98.1)
 - #162303 (Fix a minor spelling error in `library/std/src/sys/fs/unix.rs`)
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 5, 2026
…unit, r=davidtwco

fix `is_homogeneous_aggregate`: use unit size, not total size

fixes rust-lang#161382

The vector case of the three (arm, aarch64, powerpc64) `is_homogeneous_aggregate` implementations looked at the total type size, instead of the unit size.

blocked on rust-lang#161070 merging

r? davidtwco
rust-bors Bot pushed a commit that referenced this pull request Sep 5, 2026
…uwer

Rollup of 25 pull requests

Successful merges:

 - #162072 (Add new Tier-3 target: `powerpc64-sony-ps3`)
 - #162179 (type system const items via direct rhs)
 - #162255 (let people change rustc-dev-guide in-tree)
 - #162262 (Avoid manually instantiating some binders in error reporting with `-Znext-solver`)
 - #162277 (Introduce `rustc_middle::middel::resolve`)
 - #162285 (box: fixup map/try_map deallocate calls)
 - #162286 (string: don't unwind prematurely)
 - #162289 (alloc: a bunch of safety comments)
 - #162290 (abby test DSL: AliasTyOutlivesViaEnv)
 - #162292 (Update `askama` version to `0.16.1`)
 - #160509 (Remove `RegionExt`; move methods to `Region` in `rustc_type_ir`)
 - #160906 (Suggest usize instead of placeholder type for array length constants)
 - #160936 (traits: Represent live alias arguments as bitsets)
 - #161394 (fix `is_homogeneous_aggregate`: use unit size, not total size )
 - #161400 (Improve diagnostics for references to closures)
 - #161656 (Suggest mutable references for FnMut closure arguments)
 - #161711 (Add more splat fn type tests)
 - #161786 (Make `tcx.def_id_partial_cmp` public)
 - #161953 (sanitizers: Implicitly disable mutually exclusive sanitizers)
 - #162155 (add suggestion for `rustc_allowed_through_unstable_modules` attribute)
 - #162212 (Implement `Rng` for `Box`)
 - #162246 (Fix incorrect meta span)
 - #162266 (std: fix typo)
 - #162291 (Add regression test from 1.98.1)
 - #162303 (Fix a minor spelling error in `library/std/src/sys/fs/unix.rs`)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-test-infra-minicore Area: `minicore` test auxiliary and `//@ add-core-stubs` S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mishandling of AAPCS64 Homogeneous Vector Aggregates (HVAs) on aarch64-unknown-linux-gnu

6 participants