From patchwork Wed Dec 3 10:30:11 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alan Lawrence X-Patchwork-Id: 417337 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 6A72614009B for ; Wed, 3 Dec 2014 21:30:28 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:in-reply-to :content-type; q=dns; s=default; b=o9T23IcgYpu1EARYR1UVPDR/CO2uJ 2y5SiN8Fz2NCeNp3XmH0ksuF5Yrf0UDZFIB3eCbmlbi/3YH8PC9nfF6SaZYcNvTt GX7nPbnKv2IR/db3VBa/NmQmuW0KZe3Zx8ia4JGkbJHCmoVsg49VTBQkeoclFWc6 w+4VwyqemSRYbQ= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:in-reply-to :content-type; s=default; bh=/yVIgDz00wHWwYr2XsEotKpUSFw=; b=QlU ScD3BB0uEbIF+cxDCgNWu0bGN5LSMkEyojTR7dcy5EV/owxqsf9KnaZHfECZbpqa i4Z9GwEpBohKJq+c/41N9/JuyenS++7cJa1smRsRK/gREkTpPh5Jwv09sby3fwzr hMF0HMxhuII2EgFwjsduCxn+iqRyv15MRllYaySw= Received: (qmail 21515 invoked by alias); 3 Dec 2014 10:30:21 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 21497 invoked by uid 89); 3 Dec 2014 10:30:20 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: service87.mimecast.com Received: from service87.mimecast.com (HELO service87.mimecast.com) (91.220.42.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 03 Dec 2014 10:30:18 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by service87.mimecast.com; Wed, 03 Dec 2014 10:30:15 +0000 Received: from [10.1.209.51] ([10.1.255.212]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Wed, 3 Dec 2014 10:30:13 +0000 Message-ID: <547EE633.2090506@arm.com> Date: Wed, 03 Dec 2014 10:30:11 +0000 From: Alan Lawrence User-Agent: Thunderbird 2.0.0.24 (X11/20101213) MIME-Version: 1.0 To: "gcc-patches@gcc.gnu.org" CC: James Greenhalgh Subject: Re: Ping with testcase: [PATCH][AArch64] Fix __builtin_aarch64_absdi, must not fold to ABS_EXPR In-Reply-To: <20141126163550.GA26240@arm.com> X-MC-Unique: 114120310301501101 X-IsSubscribed: yes 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. diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c index c0881e643440dd688570bcbc2a849ae2d441225a..7a444053fc1aeb4f7d2764b7fd50a39fe0e4f83b 100644 --- a/gcc/config/aarch64/aarch64-builtins.c +++ b/gcc/config/aarch64/aarch64-builtins.c @@ -126,11 +126,9 @@ typedef struct enum aarch64_type_qualifiers *qualifiers; } aarch64_simd_builtin_datum; -/* The qualifier_internal allows generation of a unary builtin from - a pattern with a third pseudo-operand such as a match_scratch. */ static enum aarch64_type_qualifiers aarch64_types_unop_qualifiers[SIMD_MAX_BUILTIN_ARGS] - = { qualifier_none, qualifier_none, qualifier_internal }; + = { qualifier_none, qualifier_none }; #define TYPES_UNOP (aarch64_types_unop_qualifiers) static enum aarch64_type_qualifiers aarch64_types_unopu_qualifiers[SIMD_MAX_BUILTIN_ARGS] @@ -1275,7 +1273,7 @@ aarch64_fold_builtin (tree fndecl, int n_args ATTRIBUTE_UNUSED, tree *args, switch (fcode) { - BUILTIN_VALLDI (UNOP, abs, 2) + BUILTIN_VDQF (UNOP, abs, 2) return fold_build1 (ABS_EXPR, type, args[0]); break; VAR1 (UNOP, floatv2si, 2, v2sf) diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index 17570ba026b45df097f8838751eed86d7ce03609..25ad10405a88ba6e629bb2bf671041a639876ff2 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -1957,9 +1957,8 @@ ) (define_insn_and_split "absdi2" - [(set (match_operand:DI 0 "register_operand" "=r,w") - (abs:DI (match_operand:DI 1 "register_operand" "r,w"))) - (clobber (match_scratch:DI 2 "=&r,X"))] + [(set (match_operand:DI 0 "register_operand" "=&r,w") + (abs:DI (match_operand:DI 1 "register_operand" "r,w")))] "" "@ # @@ -1969,7 +1968,7 @@ && GP_REGNUM_P (REGNO (operands[1]))" [(const_int 0)] { - emit_insn (gen_rtx_SET (VOIDmode, operands[2], + emit_insn (gen_rtx_SET (VOIDmode, operands[0], gen_rtx_XOR (DImode, gen_rtx_ASHIFTRT (DImode, operands[1], @@ -1978,7 +1977,7 @@ emit_insn (gen_rtx_SET (VOIDmode, operands[0], gen_rtx_MINUS (DImode, - operands[2], + operands[0], gen_rtx_ASHIFTRT (DImode, operands[1], GEN_INT (63)))));