Skip to content

Add 2048-bit HvxVectorPair support to Hexagon SIMD ABI checks #152552

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rust-bors merged 1 commit into from
Feb 13, 2026
Merged

Add 2048-bit HvxVectorPair support to Hexagon SIMD ABI checks #152552

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions compiler/rustc_target/src/target_features.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -984,8 +984,11 @@ const RISCV_FEATURES_FOR_CORRECT_FIXED_LENGTH_VECTOR_ABI: &'static [(u64, &'stat
const SPARC_FEATURES_FOR_CORRECT_FIXED_LENGTH_VECTOR_ABI: &'static [(u64, &'static str)] =
&[/*(64, "vis")*/];

const HEXAGON_FEATURES_FOR_CORRECT_FIXED_LENGTH_VECTOR_ABI: &'static [(u64, &'static str)] =
&[(512, "hvx-length64b"), (1024, "hvx-length128b")];
const HEXAGON_FEATURES_FOR_CORRECT_FIXED_LENGTH_VECTOR_ABI: &'static [(u64, &'static str)] = &[
(512, "hvx-length64b"), // HvxVector in 64-byte mode
(1024, "hvx-length128b"), // HvxVector in 128-byte mode, or HvxVectorPair in 64-byte mode

@madsmtm madsmtm Feb 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Am I correctly understanding that a 1024-bit HvxVectorPair would actually require the hvx-length64b feature (and that the check is overly conservative here)?

@madsmtm madsmtm Feb 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It actually sounds like the real problem is the check itself, features_for_correct_fixed_length_vector_abi changes depending on PassMode::Pair or PassMode::Direct?

@androm3da androm3da Feb 12, 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.

It actually sounds like the real problem is the check itself, features_for_correct_fixed_length_vector_abi changes depending on PassMode::Pair or PassMode::Direct?

Hmm good point let me take a closer look.

@androm3da androm3da Feb 12, 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.

I could do this...

const HEXAGON_FEATURES_FOR_CORRECT_FIXED_LENGTH_VECTOR_ABI: &'static [(u64, &'static str)] = &[
    (512, "hvx-length64b"),
    (1024, "hvx"),           // hvx is implied by both length modes
    (2048, "hvx-length128b"),
];

... I don't love it though. Not the clearest error message.

I wonder if I could specialize like so

fn check_vector_abi_feature(&self, size_bits: u64, have_feature: impl Fn(&str) -> bool) -> Result<(), 
&'static str> {
    match size_bits {
        0..=512 => {
            if have_feature("hvx-length64b") { Ok(()) }
            else { Err("hvx-length64b") }
        }
        513..=1024 => {
            if have_feature("hvx-length64b") || have_feature("hvx-length128b") { Ok(()) }
            else { Err("hvx-length64b or hvx-length128b") }
        }
        1025..=2048 => {
            if have_feature("hvx-length128b") { Ok(()) }
            else { Err("hvx-length128b") }
        }
        _ => Err("unsupported vector size")
    }
}

@madsmtm madsmtm Feb 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm, I'm not sure. I think your proposed solution would accept in a sense "too many" ABIs, would it not? E.g. passing a 1024-bit HvxVector with only the "hvx-length64b" feature enabled should fail in a correct implementation (if I'm understanding right)?

Kinda odd though that all of this isn't a problem on other architectures, do you understand why that is? Is there something special about Hexagon's pair vectors and feature flags here?

@androm3da androm3da Feb 12, 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.

I think other architectures' target features are cumulative - +newest-whizbang implies +slightly-older-whizbang and that in turn implies +older-still-whizbang. HVX 64 byte mode and 128 byte mode are mutually exclusive. So now we have ambiguity if we're trying to map a size to a feature. 1024 bit values could either be a single 128 byte register in hvx-length128b and a 128-byte regpair in hvx-length64b. Both of these imply hvx but none of these imply the other lengths.

I don't know how the PassMode works yet - I don't think the HvxVectorPair would use PassMode::Pair but I will do some digging to try and figure it out.

@madsmtm madsmtm Feb 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah, I didn't get that they were incompatible. Must at least one of them be set? If so, then I'd lean towards (1024, "hvx"),.

I don't think the HvxVectorPair would use PassMode::Pair but I will do some digging to try and figure it out.

Oh, okay.

(2048, "hvx-length128b"), // HvxVectorPair in 128-byte mode
];
const MIPS_FEATURES_FOR_CORRECT_FIXED_LENGTH_VECTOR_ABI: &'static [(u64, &'static str)] =
&[(128, "msa")];
const CSKY_FEATURES_FOR_CORRECT_FIXED_LENGTH_VECTOR_ABI: &'static [(u64, &'static str)] =
Expand Down
Loading