From patchwork Wed Feb 11 10:57:03 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alan Lawrence X-Patchwork-Id: 438743 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 46F521401F6 for ; Wed, 11 Feb 2015 22:01:32 +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:references :in-reply-to:content-type; q=dns; s=default; b=b7CvhGhjPt/mlE2Sd +njbGJIy8aQSmhdbZi/Z6hX/lnXVd/g2ifLmn7pvtCbtDEOZpHOQsKUAmGqZ9aFk 0SEs/bsauNh+r9+3M/NkTVKZVj+DLfw/KM2FSwLWoKQgJlsBraN85qeBsfFSBP2S iUWlyxqghkO4ZkOwGZJMCmBzF4= 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:references :in-reply-to:content-type; s=default; bh=Bu9f9qxhDelSShEaZ+Pt8LT 1/lM=; b=RtqePOL0+STg6sSVCaDs7nGNF+YYjn8duBzSvx5X73Fz/91Bphumb0f Le/w1dASTqA8/cipf+07IvhC5yP9hkMx+H8v3MnR17W3NV+bwvkx5WBfFHUPCn2X A8UEodFqVB4mTosWZcXwI+9/Lqga69X+jmfhOJ7EwKxOMqQKx9BM= Received: (qmail 13914 invoked by alias); 11 Feb 2015 10:57:09 -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 13890 invoked by uid 89); 11 Feb 2015 10:57:08 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 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, 11 Feb 2015 10:57:07 +0000 Received: from cam-owa2.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by service87.mimecast.com; Wed, 11 Feb 2015 10:57:04 +0000 Received: from [10.2.207.65] ([10.1.255.212]) by cam-owa2.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Wed, 11 Feb 2015 10:57:02 +0000 Message-ID: <54DB357F.5090709@arm.com> Date: Wed, 11 Feb 2015 10:57:03 +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 , Marcus Shawcroft Subject: Re: [PATCH][AArch64] Fix illegal assembly 'eon v1, v2, v3' References: <54C8D6ED.7050206@arm.com> <20150128140404.GA29191@arm.com> <20150210091357.GA20593@arm.com> In-Reply-To: <20150210091357.GA20593@arm.com> X-MC-Unique: 115021110570418601 X-IsSubscribed: yes Here is a revised patch using '#'. Bootstrap + check-gcc natively on aarch64-none-linux-gnu. My feeling is still against including the testcase because even when it passes it doesn't increase my confidence that the compiler is right very much (i.e. the insn could still be reading which_alternative at split time, but which_alternative just happened to have a useful value in it - as was happening in existing test gcc.target/aarch64/eon_1.c). I've recorded the testcase in PR/64997, however. If the maintainers feel the testcase should be included, then I could prepare that as a separate patch? gcc/ChangeLog: PR target/64997 * config/aarch64/aarch64.md (*xor_one_cmpl3): Use FP_REGNUM_P as split condition; force split via '#' in output pattern. --Alan James Greenhalgh wrote: > On Wed, Jan 28, 2015 at 02:04:04PM +0000, James Greenhalgh wrote: >> On Wed, Jan 28, 2015 at 12:32:45PM +0000, Alan Lawrence wrote: >>> Ok for stage 4? >> This is a regression from 4.9, so once we iron out some nits, it should >> be. >> >>> gcc/ChangeLog: >>> >>> * config/aarch64/aarch64.md (*xor_one_cmpl3): Use FP_REGNUM_P >>> as split condition. >> And a testcase, please! >> >>> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md >>> index bc49fbe68a978b3ca069c6d084f542773df84bcb..d4b3f7b03ba0ab570cec5ce862e8c5f38f417ed1 100644 >>> --- a/gcc/config/aarch64/aarch64.md >>> +++ b/gcc/config/aarch64/aarch64.md >>> @@ -3054,7 +3054,7 @@ >>> (match_operand:GPI 2 "register_operand" "r,w"))))] >>> "" >>> "eon\\t%0, %1, %2" ;; For GPR registers (only). >> This should be: >> "@ >> eon\\t%0, %1, %2 >> #" >> >> which would have forced a split. >> >> Your patch is useful regardless, as I guess we could have ended up >> needlessly splitting if we got unlucky with whatever had been left >> in which_alternative. > > Hi Alan, > > Do you have any plans to respin this patch? I'd like to see it fixed > for GCC 5.0 if possible. > > Thanks, > James > diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index bc49fbe..9f08046 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -3053,8 +3053,10 @@ (not:GPI (xor:GPI (match_operand:GPI 1 "register_operand" "r,?w") (match_operand:GPI 2 "register_operand" "r,w"))))] "" - "eon\\t%0, %1, %2" ;; For GPR registers (only). - "reload_completed && (which_alternative == 1)" ;; For SIMD registers. + "@ + eon\\t%0, %1, %2 + #" + "reload_completed && FP_REGNUM_P (REGNO (operands[0]))" ;; For SIMD registers. [(set (match_operand:GPI 0 "register_operand" "=w") (xor:GPI (match_operand:GPI 1 "register_operand" "w") (match_operand:GPI 2 "register_operand" "w")))