My comments inlined ...

On Wed, Oct 22, 2025 at 3:37 PM Luis Silva <[email protected]> wrote:
>
> From: Loeka Rogge <[email protected]>
>
> V2HI vectors, explicitly or auto-generated, could be stored in memory wrongly
> due to endianness. For example in the following c code stores to the struct
> are SLP vectorized, causing them to be stored in the wrong order:
>
> >   struct S {short a; short b;};
> >   s.a = 520;
> >   s.b = -1;
>
> in the split2 pass the following register set:
>
> > (const_vector:V2HI [
> >         (const_int 520 [0x208])
> >         (const_int -1 [0xffffffffffffffff])
> >     ])) "smallTest.c":16:9 484 {*movv2hi_insn}
>
> is converted to:
>
> > (const_int -65016 [0xffffffffffff0208])) "smallTest.c":16:9 3 {*movsi_insn}
>
> and is then loaded into the struct. For big-endian this is wrong because
> the most significant bytes are written first in memory, storing -1 instead of
> 520 in s.a .
> This patch swaps the 2 values in this step if the target is big-endian.
> The added test creates a vector of 2 shorts and verifies the order when
> it is passed in a register or in memory.
>
> Regtested for arc and big-endian arc.
>
> gcc/ChangeLog:
>
>         * config/arc/simdext.md: Change order for movv2hi for big-endian.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/arc/movv2hi-be.c: New test.
>
> Signed-off-by: Loeka Rogge <[email protected]>
> ---
>  gcc/config/arc/simdext.md                 | 11 ++++----
>  gcc/testsuite/gcc.target/arc/movv2hi-be.c | 32 +++++++++++++++++++++++
>  2 files changed, 38 insertions(+), 5 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/arc/movv2hi-be.c
>
> diff --git a/gcc/config/arc/simdext.md b/gcc/config/arc/simdext.md
> index a53b2ba737..94ea0884b7 100644
> --- a/gcc/config/arc/simdext.md
> +++ b/gcc/config/arc/simdext.md
> @@ -1438,11 +1438,12 @@
>    "reload_completed && GET_CODE (operands[1]) == CONST_VECTOR"
>    [(set (match_dup 0) (match_dup 2))]
>    {
> -   HOST_WIDE_INT intval = INTVAL (XVECEXP (operands[1], 0, 1)) << 16;
> -   intval |= INTVAL (XVECEXP (operands[1], 0, 0)) & 0xFFFF;
> -
> -   operands[0] = gen_rtx_REG (SImode, REGNO (operands[0]));
> -   operands[2] = GEN_INT (trunc_int_for_mode (intval, SImode));
> +    int hi = !TARGET_BIG_ENDIAN;
> +    int lo = !hi;

I know there are code snips using the above constructs, but I don't
think it is safe. Please use:
hi = TARGET_BIG_ENDIAN ? 0 : ...
lo = ....

> +    HOST_WIDE_INT intval = INTVAL (XVECEXP (operands[1], 0, hi)) << 16;
> +    intval |= INTVAL (XVECEXP (operands[1], 0, lo)) & 0xFFFF;

maybe u want to do something like:
hi_val = INTVAL (XVECEXP (operands[1], 0, hi))
lo_val = INTVAL (XVECEXP (operands[1], 0, lo))
hi_val = zext_hwi (hi_val, 16);
lo_val = zext_hwi (lo_val, 16);
intval = lo_val | (hi_val << 16);

> +    operands[0] = gen_rtx_REG (SImode, REGNO (operands[0]));
> +    operands[2] = GEN_INT (trunc_int_for_mode (intval, SImode));

Also, please make sure your patch applies on top of the master branch.

Cheers,
Claudiu

Reply via email to