This is the mail archive of the
mailing list for the GCC project.
Re: [AArch64] Fix integer vabs intrinsics
- From: Andrew Pinski <pinskia at gmail dot com>
- To: James Greenhalgh <james dot greenhalgh at arm dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Marcus Shawcroft <marcus dot shawcroft at arm dot com>
- Date: Fri, 2 May 2014 02:00:15 -0700
- Subject: Re: [AArch64] Fix integer vabs intrinsics
- Authentication-results: sourceware.org; auth=none
- References: <1399020521-4506-1-git-send-email-james dot greenhalgh at arm dot com>
On Fri, May 2, 2014 at 1:48 AM, James Greenhalgh
> 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
In fact the ABS rtl code is not undefined for the overflow.
> We keep the standard pattern name around for the benefit of
> 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?
> 2014-05-02 James Greenhalgh <firstname.lastname@example.org>
> * 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.