On Fri, May 2, 2014 at 1:48 AM, James Greenhalgh <james.greenha...@arm.com> wrote: > > Hi, > > Unlike the mid-end's concept of an ABS_EXPR, which treats overflow as > undefined/impossible, the neon intrinsics vabs intrinsics should behave as > the hardware. That is to say, the pseudo-code sequence:
Only for signed integer types. You should be able to use an unsigned integer type here instead. > > a = vabs_s8 (vdup_n_s8 (-128)); > assert (a >= 0); > > does not hold. As in hardware > > abs (-128) == -128 > > Folding vabs intrinsics to an ABS_EXPR is thus a mistake, and we should avoid > it. In fact, we have to be even more careful than that, and keep the integer > vabs intrinsics as an unspec in the back end. No it is not. The mistake is to use signed integer types here. Just add a conversion to an unsigned integer vector and it will work correctly. In fact the ABS rtl code is not undefined for the overflow. Thanks, Andrew Pinski > > We keep the standard pattern name around for the benefit of > auto-vectorization. > > Tested on aarch64-none-elf with no issues. > > This will also be a bug on 4.9 (ugh), OK for trunk and gcc-4_9-branch? > > Thanks, > James > > --- > 2014-05-02 James Greenhalgh <james.greenha...@arm.com> > > * config/aarch64/aarch64-builtins.c (aarch64_fold_builtin): Don't > fold integer abs builtins. > * config/aarch64/aarch64-simd-builtins.def (abs): Split by integer > and floating point variants. > * config/aarch64/aarch64-simd.md (aarch64_abs<mode>): New. > * config/aarch64/iterators.md (unspec): Add UNSPEC_ABS.
