Hi, On Thu, 14 Aug 2025 at 11:27, Christophe Lyon <christophe.l...@linaro.org> wrote: > > 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. >
Hi, My todo-list including re-implementing these MVE intrinsics using the same framework as the others, and I have just posted a series which does that: https://inbox.sourceware.org/gcc-patches/20250827144542.646760-1-christophe.l...@linaro.org/T/ patches 3, 4 and 5 should fix asrl and lsll. Thanks, Christophe > 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 > >
