This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Ping with testcase: [PATCH][AArch64] Fix __builtin_aarch64_absdi, must not fold to ABS_EXPR
- From: Marcus Shawcroft <marcus dot shawcroft at gmail dot com>
- To: Alan Lawrence <alan dot lawrence at arm dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 8 Dec 2014 13:34:34 +0000
- Subject: Re: Ping with testcase: [PATCH][AArch64] Fix __builtin_aarch64_absdi, must not fold to ABS_EXPR
- Authentication-results: sourceware.org; auth=none
- References: <20141126163550 dot GA26240 at arm dot com> <547EE633 dot 2090506 at arm dot com>
On 3 December 2014 at 10:30, Alan Lawrence <alan.lawrence@arm.com> wrote:
> On Wed, Nov 26, 2014 at 04:35:50PM +0000, James Greenhalgh wrote:
>> Why do we want to turn off folding for the V4SF/V2SF/V2DF modes of these
>> intrinsics? There should be no difference between the mid-end definition
>> and the intrinsic definition of their behaviour.
>
> Good point. Done.
>
>> I also note that the integer forms of these now end up as an "abs" RTL
>> expression - can we guarantee that preserves the intrinsics behaviour and
>> no RTL-folder will come along and mis-optimize? Documentation is vague
>> on this point.
>
> I don't think we can guarantee that indefinitely, but it doesn't seem anyone
> has implemented such a rule *at present*. And we'd lose e.g. constant
> folding, if we switched to using an AArch64-specific UNSPEC. If anyone does
> add such a rule, I'd have some hope that the testcase will catch it...
>
>> I'm also not convinced by the SIMD_ARG_SCRATCH foo you add. Looking at
>> the aarch64.md:absdi2 pattern I can't see why we need that scratch at
>> all. It seems we could get away with marking operand 0 early-clobber and
>> using it as our scratch register. Then we could drop all the extra
>> infrastructure from this patch.
>
> Hah, good point. Ok, I attach a revised patch, updating the pattern as you
> suggest and omitting the infrastructure changes. (I'm leaving the now-unused
> qualifier_internal as it stands, as there are arguments both for removing it
> and "fixing" it as per previous patch, but I don't think we should hold this
> up in the case that we want to have that discussion!)
>
> Cross-tested check-gcc on aarch64-none-elf, including previously-posted test
> of vabs_s8.
>
> Ok for trunk (with previously-posted testcase) ?
> --Alan
>
> gcc/ChangeLog:
>
> * config/aarch64/aarch64.md (absdi2): Remove scratch operand by
> earlyclobbering result operand.
>
> * config/aarch64/aarch64-builtins.c (aarch64_types_unop_qualifiers):
> Remove final qualifier_internal.
> (aarch64_fold_builtin): Stop folding abs builtins, except on floats.
OK, and the test case too /Marcus