From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 106695 invoked by alias); 19 Jun 2017 16:25:04 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 106617 invoked by uid 89); 19 Jun 2017 16:25:03 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 19 Jun 2017 16:25:01 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id DAAB880D; Mon, 19 Jun 2017 09:25:04 -0700 (PDT) Received: from [10.2.207.77] (e100706-lin.cambridge.arm.com [10.2.207.77]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 6636A3F587; Mon, 19 Jun 2017 09:25:03 -0700 (PDT) Message-ID: <5947FADD.2020407@foss.arm.com> Date: Mon, 19 Jun 2017 16:25:00 -0000 From: Kyrill Tkachov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: James Greenhalgh CC: gcc-patches@gcc.gnu.org, nd@arm.com, richard.earnshaw@arm.com, ramana.radhakrishnan@arm.com, nickc@redhat.com Subject: Re: [Patch ARM] Fix PR71778 References: <59410E2A.1020808@foss.arm.com> <1497604036-4653-1-git-send-email-james.greenhalgh@arm.com> <5943ADED.10706@foss.arm.com> <20170619161650.GA24181@arm.com> In-Reply-To: <20170619161650.GA24181@arm.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2017-06/txt/msg01339.txt.bz2 On 19/06/17 17:16, James Greenhalgh wrote: > On Fri, Jun 16, 2017 at 11:07:41AM +0100, Kyrill Tkachov wrote: >> On 16/06/17 10:07, James Greenhalgh wrote: >>> On Wed, Jun 14, 2017 at 11:21:30AM +0100, Kyrill Tkachov wrote: >>> >>> <...> >>> >>>> That movv2di expander is the one in vec-common.md that ends up calling >>>> neon_make_constant. I wonder why const0_rtx passed its predicate check >>>> (that would require a V2DImode vector of zeroes rather than a const0_rtx). >>>> Perhaps the midend code at this point doesn't check the operand predicate. >>>> >>>> In the builtin expansion code that you quoted I wonder wonder if we could fail >>>> more gracefully by returning CONST0_RTX (mode[argc]) to match the expected >>>> mode of the operand (we've already emitted an error, so we shouldn't care >>>> what RTL we emit as long as it doesn't cause an ICE). >>> <...> >>> >>>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c >>>> index e503891..b8d59c6 100644 >>>> --- a/gcc/config/arm/arm.c >>>> +++ b/gcc/config/arm/arm.c >>>> @@ -12124,6 +12124,11 @@ neon_make_constant (rtx vals) >>>> if (n_const == n_elts) >>>> const_vec = gen_rtx_CONST_VECTOR (mode, XVEC (vals, 0)); >>>> } >>>> + else if (vals == const0_rtx) >>>> + /* Something invalid, perhaps from expanding an intrinsic >>>> + which requires a constant argument, where a variable argument >>>> + was passed. */ >>>> + return const0_rtx; >>>> else >>>> gcc_unreachable (); >>>> >>>> I'm not a fan of this as the function has a precondition that its argument is >>>> a PARALLEL or a CONST_VECTOR and special-casing const0_rtx breaks that. I'd >>>> rather we tried fixing this closer to the error source. Can you try the >>>> suggestion above instead please? >>> Your suggestion doesn't quite work, but this is pretty close to it. Rather >>> than try to guess at the correct mode for CONST0_RTX (we can't just use >>> mode[argc] as that will get you the scalar mode), we can just return target >>> directly. That will ensure we've given something valid back in the correct >>> mode, even if it is not all that useful. >> Yeah, that actually looks better. >> >>> Bootstrapped on arm-none-linux-gnueabihf. OK? >> Ok. > Thanks. > > The patch applies cleanly to gcc-7-branch and gcc-6-branch, both of which > I've bootstrapped and tested on arm-none-linux-gnueabihf without issue. > > Is it OK for me to apply these backports and close out the PR (it is > marked as a 6/7 regression). Ok. Thanks, Kyrill > Thanks, > James > > >>> --- >>> gcc/ >>> >>> 2017-06-15 James Greenhalgh >>> >>> PR target/71778 >>> * config/arm/arm-builtins.c (arm_expand_builtin_args): Return TARGET >>> if given a non-constant argument for an intrinsic which requires a >>> constant. >>> >>> gcc/testsuite/ >>> >>> 2017-06-15 James Greenhalgh >>> >>> PR target/71778 >>> * gcc.target/arm/pr71778.c: New. >>>