From patchwork Fri Jan 6 16:45:46 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kyrill Tkachov X-Patchwork-Id: 90205 Delivered-To: patch@linaro.org Received: by 10.140.20.101 with SMTP id 92csp9527141qgi; Fri, 6 Jan 2017 08:46:46 -0800 (PST) X-Received: by 10.84.133.1 with SMTP id 1mr3926694plf.151.1483721206209; Fri, 06 Jan 2017 08:46:46 -0800 (PST) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id s137si79916430pfs.170.2017.01.06.08.46.45 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 06 Jan 2017 08:46:46 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-return-445577-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org; spf=pass (google.com: domain of gcc-patches-return-445577-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-445577-patch=linaro.org@gcc.gnu.org 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=pFJaDKmkhtnu9eOJi 9Vxim71dq0waIDsSsYG8VXf8f0sTMJ2MtdyP6tclsNyxrtUcDbwx8zXeT9Za61RA VQK6d1EYLFtfyTVppOVVeVjJqbKto4mFOmN6JdViQRyq7ghzOgDK/BLOaz2jlWdS UrmihAOErioSOGeXX4fbCF3COo= 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=SsF1dr8/sIQmUD+Vmb+7T/p oPd8=; b=vm+k9+FkWWEhiBkl/+n/IC8BAwjTWatE7QPzklNegfjjD2MmtSBcv65 C5iYnh9/wbYTv+Uof62vWU+vGdohoEi0ZNqnLVqnaSCdx5qTsD3vdzlxqNLcr3SL T5hPLzEnZqtlpKyKr578D4uBI2F64d73vLCgea7lJYiYCZFHgQX8= Received: (qmail 14968 invoked by alias); 6 Jan 2017 16:46:01 -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 14816 invoked by uid 89); 6 Jan 2017 16:46:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.1 required=5.0 tests=BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=H*i:sk:586E376, H*f:sk:586E376, H*MI:sk:586E376 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; Fri, 06 Jan 2017 16:45:50 +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 55D8E707; Fri, 6 Jan 2017 08:45:48 -0800 (PST) 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 D90BD3F242; Fri, 6 Jan 2017 08:45:47 -0800 (PST) Message-ID: <586FC9BA.7020608@foss.arm.com> Date: Fri, 06 Jan 2017 16:45:46 +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: Richard Biener CC: GCC Patches Subject: Re: [PATCH][tree-ssa-address] Use simplify_gen_binary in gen_addr_rtx References: <585955CF.3010501@foss.arm.com> <1F1B16CA-0488-4666-9EA8-2B68A55C66C5@gmail.com> <585A4DFF.8080202@foss.arm.com> <586D0FC9.2000403@foss.arm.com> <586E3760.1060008@foss.arm.com> In-Reply-To: <586E3760.1060008@foss.arm.com> On 05/01/17 12:09, Kyrill Tkachov wrote: > > On 05/01/17 12:01, Richard Biener wrote: >> On Wed, Jan 4, 2017 at 4:07 PM, Kyrill Tkachov >> wrote: >>> On 04/01/17 14:19, Richard Biener wrote: >>>> On Wed, Dec 21, 2016 at 10:40 AM, Kyrill Tkachov >>>> wrote: >>>>> On 20/12/16 17:30, Richard Biener wrote: >>>>>> On December 20, 2016 5:01:19 PM GMT+01:00, Kyrill Tkachov >>>>>> wrote: >>>>>>> Hi all, >>>>>>> >>>>>>> The testcase in this patch generates bogus assembly for arm with -O1 >>>>>>> -mfloat-abi=soft: >>>>>>> strd r4, [#0, r3] >>>>>>> >>>>>>> This is due to non-canonical RTL being generated during expansion: >>>>>>> (set (mem:DI (plus:SI (const_int 0 [0]) >>>>>>> (reg/f:SI 153)) [0 MEM[symbol: a, index: _26, offset: 0B]+0 S8 >>>>>>> A64]) >>>>>>> (reg:DI 154)) >>>>>>> >>>>>>> Note the (plus (const_int 0) (reg)). This is being generated in >>>>>>> gen_addr_rtx in tree-ssa-address.c >>>>>>> where it creates an explicit PLUS rtx through gen_rtx_PLUS, which >>>>>>> doesn't try to canonicalise its arguments >>>>>>> or simplify. The correct thing to do is to use simplify_gen_binary that >>>>>>> will handle all this properly. >>>>>> But it has to match up the validity check which passes down exactly the >>>>>> same RTL(?) Or does this stem from propagation simplifying a MEM after >>>>>> IVOPTs? >>>>> >>>>> You mean TARGET_LEGITIMATE_ADDRESS_P? Yes, it gets passed on to that, but >>>>> the arm implementation of that >>>>> doesn't try to handle non-canonical RTL (plus (const0_rtx) (reg) is not >>>>> canonical). >>>>> Or do you mean some other check? >>>> Ok, now looking at the actual patch and the code in question. For your >>>> testcase >>>> it happens that symbol == const0_rtx? In this case please amend the >>>> if (symbol) check like we do for the base, thus >>>> >>>> if (symbol && symbol != const0_rtx) >>> >>> No, symbol is not const0_rtx (it's just a symbol_ref). >>> index is const0_rtx and so gets assigned to *addr at the beginning of the >>> function. >>> base and step are NULL_RTX. >>> So at the time of the check: >>> if (*addr) >>> *addr = gen_rtx_PLUS (address_mode, *addr, act_elem); >>> else >>> *addr = act_elem; >>> >>> *addr is const0_rtx. Do you want to amend that check to: >>> if (*addr && *addr != const0_rtx) ? >> Hmm, I guess given the if (step) in if (index) *addr can end up being >> a not simplified mult. So instead do >> >> if (index && index != const0_rtx) > > That works, I'll test a patch for this. > Here it is. Bootstrapped and tested on arm-none-linux-gnueabihf and aarch64-none-linux-gnu. Ok? Thanks, Kyrill 2017-01-06 Kyrylo Tkachov * tree-ssa-address.c (gen_addr_rtx): Don't handle index if it is const0_rtx. 2017-01-06 Kyrylo Tkachov * gcc.dg/20161219.c: New test. >>> I haven't looked where the const0_rtx index comes from, so I don't know if >>> it >>> could have other CONST_INT values that may cause trouble. >> Usually this happens when constant folding / propagation happens after >> IVOPTs generates the TARGET_MEM_REF. We do have some canonicalization >> foldings for TMR, see maybe_fold_tmr, but that should have made index NULL >> if it was constant... So maybe we fail to fold a stmt at some point. >> >> Btw, I fail to see the bogus asm with my arm-non-eabi cross with -O >> -mfloat-abi=soft >> so I can't tell how the TMR arrives at RTL expansion. > > You'll also want to specify -marm (this doesn't trigger on Thumb) and perhaps -march=armv7-a. > > Thanks, > Kyrill > >> Richard. >> >>> Kyrill >>> >>> >>>> Richard. >>>> >>>>> Thanks, >>>>> Kyrill >>>>> >>>>> >>>>>>> I didn't change the other gen_rtx_PLUS calls in this function as their >>>>>>> results is later used in XEXP operations >>>>>>> that seem to rely on a PLUS expression being explicitly produced, but >>>>>>> this particular call doesn't, so it's okay >>>>>>> to change it. With this patch the sane assembly is generated: >>>>>>> strd r4, [r3] >>>>>>> >>>>>>> Bootstrapped and tested on arm-none-linux-gnueabihf, x86_64, >>>>>>> aarch64-none-linux-gnu. >>>>>>> >>>>>>> Ok for trunk? >>>>>>> >>>>>>> Thanks, >>>>>>> Kyrill >>>>>>> >>>>>>> 2016-12-20 Kyrylo Tkachov >>>>>>> >>>>>>> * tree-ssa-address.c (gen_addr_rtx): Use simplify_gen_binary to >>>>>>> add >>>>>>> *addr to act_elem. >>>>>>> >>>>>>> 2016-12-20 Kyrylo Tkachov >>>>>>> >>>>>>> * gcc.dg/20161219.c: New test. >>>>>> > diff --git a/gcc/testsuite/gcc.dg/20161219.c b/gcc/testsuite/gcc.dg/20161219.c new file mode 100644 index 0000000000000000000000000000000000000000..93ea8d2364d9ab54704a84e6c0bff0427df82db8 --- /dev/null +++ b/gcc/testsuite/gcc.dg/20161219.c @@ -0,0 +1,30 @@ +/* { dg-do assemble } */ +/* { dg-options "-O1 -w" } */ + +static long long a[9]; +int b, c, d, e, g; + +static int +fn1 (int *p1) +{ + b = 1; + for (; b >= 0; b--) + { + d = 0; + for (;; d++) + { + e && (a[d] = 0); + if (*p1) + break; + c = (int) a; + } + } + return 0; +} + +int +main () +{ + int f = fn1 ((int *) f); + return f; +} diff --git a/gcc/tree-ssa-address.c b/gcc/tree-ssa-address.c index 3e3cad150b64e813509e079f9ea91d65806e414a..8d46a3e67337dd7639d0b17ca888f50009d65b93 100644 --- a/gcc/tree-ssa-address.c +++ b/gcc/tree-ssa-address.c @@ -115,7 +115,7 @@ gen_addr_rtx (machine_mode address_mode, if (offset_p) *offset_p = NULL; - if (index) + if (index && index != const0_rtx) { act_elem = index; if (step)