Skip to content

Saturating casts for integers, like _mm_packus_epi16 and _mm_packs_epi16 #369

@okaneco

Description

@okaneco
Contributor

My most common use case is for saturating narrowing casts, such as from i16 to u8. It would be great to take advantage of native instructions without first having to use simd_clamp. saturating_cast would reduce code and chances of errors from clamping with incorrect values.

I'm not sure about the availability of this feature on other architectures. It might make sense to split this for narrowing and widening features based on availability. I'm not familiar with other architectures, but it looks like ARM has narrowing casts with VQMOVN/VQMOVUN.

Perhaps there could be a fallback which does simd_clamp().cast() (or other optimal construction) for architectures that don't have these instructions.

Activity

added
C-feature-requestCategory: a feature request, i.e. not implemented / a PR
on Oct 6, 2023
calebzulawski

calebzulawski commented on Oct 7, 2023

@calebzulawski
Member

Is there a saturating cast for regular (scalar) integers? Not that that's a requirement, but it's a good guide as to what to include. I'm not opposed to adding it, though. It's worth noting that simd_clamp().cast() does the right thing on both x86-64 and aarch64: godbolt

okaneco

okaneco commented on Oct 7, 2023

@okaneco
ContributorAuthor

Not that I'm aware of in std but I wish scalars had this, as I need it a lot in contexts like image processing.

Part of what motivated this issue was that I was using i16x4/u8x4 on x86-64 with the base feature profile. I noticed that it didn't produce similar code to the i16x8 example without some coaxing (which seems to pessimize the aarch output).
https://rust.godbolt.org/z/edxro99e3

calebzulawski

calebzulawski commented on Oct 7, 2023

@calebzulawski
Member

That's definitely an LLVM issue, I'm not sure why it only happens with the 4-length vectors. I opened the linked issue to track this. I reviewed the LLVM source and simd_clamp is the right way to indicate saturation:

/// Detect patterns of truncation with unsigned saturation:
///
/// 1. (truncate (umin (x, unsigned_max_of_dest_type)) to dest_type).
///   Return the source value x to be truncated or SDValue() if the pattern was
///   not matched.
///
/// 2. (truncate (smin (smax (x, C1), C2)) to dest_type),
///   where C1 >= 0 and C2 is unsigned max of destination type.
///
///    (truncate (smax (smin (x, C2), C1)) to dest_type)
///   where C1 >= 0, C2 is unsigned max of destination type and C1 <= C2.
///
///   These two patterns are equivalent to:
///   (truncate (umin (smax(x, C1), unsigned_max_of_dest_type)) to dest_type)
///   So return the smax(x, C1) value to be truncated or SDValue() if the
///   pattern was not matched.
static SDValue detectUSatPattern(SDValue In, EVT VT, SelectionDAG &DAG,
                                 const SDLoc &DL) {
okaneco

okaneco commented on Oct 8, 2023

@okaneco
ContributorAuthor

Thanks for that.
It's good to know that what I saw was an exception with the extra instructions.

I've possibly come across another issue with 4-vectors and casting from i16x4 to u8x4. If I use a temporary array and manually unroll the cast into that destination, it produces vector instructions.
https://rust.godbolt.org/z/WrKdjx9sa

Feel free to move this to its own issue if needed.

okaneco

okaneco commented on Mar 1, 2024

@okaneco
ContributorAuthor

Both optimization patterns have now been fixed upstream. The saturating truncating cast fix is in LLVM 18, available on nightly
https://rust.godbolt.org/z/edxro99e3
The truncating cast issue was recently fixed and will probably be in LLVM 19 (above, https://rust.godbolt.org/z/WrKdjx9sa)
https://llvm.godbolt.org/z/hTbnadsnj

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    C-feature-requestCategory: a feature request, i.e. not implemented / a PR

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @calebzulawski@okaneco

        Issue actions

          Saturating casts for integers, like `_mm_packus_epi16` and `_mm_packs_epi16` · Issue #369 · rust-lang/portable-simd