Hi Keith,
On Tue, 12 Aug 2025 at 18:33, Keith Packard <kei...@keithp.com> wrote:
>
> The C shift operators do not precisely match the associated ARM
> instructions: shifts of negative values or by negative amounts are
> undefined behavior in C, and GCC may substitute alternate instruction
> sequences when it can determine that the application is using UB.
>
> Replace C shift operators with inline asm to ensure the lsll and asrl
> primitives always emit the indicated instruction.
>
> This issue caused mis-compilation of the ARM cmsis-dsp code for
> arm_biquad_cascade_df1_32x64_q31:
>
> https://github.com/ARM-software/CMSIS-DSP/pull/265
>
Thanks for the report and for the patch.
It looks almost OK, but instead of using __asm__, could you use
builtins like we do a few lines below (see uqrshll for instance) ?
In addition, could you add a testcase?
Finally, please include a ChangeLog entry in your commit message.
Thanks,
Christophe
> Signed-off-by: Keith Packard <kei...@keithp.com>
> ---
> gcc/config/arm/arm_mve.h | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/config/arm/arm_mve.h b/gcc/config/arm/arm_mve.h
> index ee18a4714fb..c4fad3c6429 100644
> --- a/gcc/config/arm/arm_mve.h
> +++ b/gcc/config/arm/arm_mve.h
> @@ -258,14 +258,16 @@ __extension__ extern __inline uint64_t
> __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
> __arm_lsll (uint64_t value, int32_t shift)
> {
> - return (value << shift);
> + __asm__("lsll %Q0, %R0, %1" : "+r" (value) : "rPg" (shift));
> + return value;
> }
>
> __extension__ extern __inline int64_t
> __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
> __arm_asrl (int64_t value, int32_t shift)
> {
> - return (value >> shift);
> + __asm__("asrl %Q0, %R0, %1" : "+r" (value) : "rPg" (shift));
> + return value;
> }
>
> __extension__ extern __inline uint64_t
> --
> 2.49.0
>
