From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 129365 invoked by alias); 1 May 2015 15:18:34 -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 127127 invoked by uid 89); 1 May 2015 15:18:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: gate.crashing.org Received: from gate.crashing.org (HELO gate.crashing.org) (63.228.1.57) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Fri, 01 May 2015 15:18:29 +0000 Received: from gate.crashing.org (localhost.localdomain [127.0.0.1]) by gate.crashing.org (8.14.1/8.13.8) with ESMTP id t41FI7iw002276; Fri, 1 May 2015 10:18:10 -0500 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id t41FI30j002263; Fri, 1 May 2015 10:18:03 -0500 Date: Fri, 01 May 2015 15:18:00 -0000 From: Segher Boessenkool To: "Kumar, Venkataramanan" Cc: "Jeff Law (law@redhat.com)" , "gcc-patches@gcc.gnu.org" , "maxim.kuvyrkov@linaro.org" Subject: Re: [RFC]: Remove Mem/address type assumption in combiner Message-ID: <20150501151758.GA1182@gate.crashing.org> References: <7794A52CE4D579448B959EED7DD0A4723DCE9F34@satlexdag06.amd.com> <20150429170335.GB21715@gate.crashing.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150429170335.GB21715@gate.crashing.org> User-Agent: Mutt/1.4.2.3i X-IsSubscribed: yes X-SW-Source: 2015-05/txt/msg00063.txt.bz2 On Wed, Apr 29, 2015 at 12:03:35PM -0500, Segher Boessenkool wrote: > On Wed, Apr 29, 2015 at 09:25:21AM +0000, Kumar, Venkataramanan wrote: > > diff --git a/gcc/combine.c b/gcc/combine.c > > index 5c763b4..945abdb 100644 > > --- a/gcc/combine.c > > +++ b/gcc/combine.c > > @@ -7703,8 +7703,6 @@ make_compound_operation (rtx x, enum rtx_code in_code) > > but once inside, go back to our default of SET. */ > > > > next_code = (code == MEM ? MEM > > - : ((code == PLUS || code == MINUS) > > - && SCALAR_INT_MODE_P (mode)) ? MEM > > : ((code == COMPARE || COMPARISON_P (x)) > > && XEXP (x, 1) == const0_rtx) ? COMPARE > > : in_code == COMPARE ? SET : in_code); > > > > > > On X86_64, it passes bootstrap and regression tests. > > But on Aarch64 the test in PR passed, but I got a few test case failures. > > > There are few patterns based on multiplication operations in Aarch64 backend which are used to match with the pattern combiner generated. > > Now those patterns have to be fixed to use SHIFTS. Also need to see any impact on other targets. > > Right. It would be good if you could find out for what targets it matters. > The thing is, if a target expects only the patterns as combine used to make > them, it will regress (as you've seen on aarch64); but it will regress > _silently_. Which isn't so nice. > > > But before that I wanted to check if the assumption in combiner, can simply be removed ? > > Seeing for what targets / patterns it makes a difference would tell us the > answer to that, too :-) > > I'll run some tests with and without your patch. So I ran the tests. These are text sizes for vmlinux built for most architectures (before resp. after the patch). Results are good, but they show many targets need some work... BIGGER 5410496 5432744 alpha 3848987 3849143 arm 2190276 2190292 blackfin 2188431 2191503 c6x (but data shrank by more than text grew) 2212322 2212706 cris 10896454 10896965 i386 7471891 7488771 parisc 6168799 6195391 parisc64 1545840 1545872 shnommu 1946649 1954149 xtensa I looked at some of those. Alpha regresses with s4add things, arm with add ,lsl things, parisc with shladd things. I tried to fix the parisc one (it seemed simplest), and the 32-bit kernel got most (but not all) of the size difference back; and the 64-bit wouldn't build (an assert in the kernel failed, and it uses a floating point reg where an integer one is needed -- I gave up). SMALLER 8688171 8688003 powerpc 20252605 20251797 powerpc64 11425334 11422558 s390 12300135 12299767 x86_64 The PowerPC differences are mostly what first was rlwinm;addi;rlwinm;lwz and now is rlwinm;lwz; i.e. the add is moved after the two rotates, the rotates are merged, and the add is made part of the offset in the load. NO DIFF 3321492 3321492 frv 3252747 3252747 m32r 4708232 4708232 microblaze 3949145 3949145 mips 5693019 5693019 mips64 2373441 2373441 mn10300 6489846 6489846 sh64 3733749 3733749 sparc 6075122 6075122 sparc64 Also quite a few without any diff. Good :-) Some of those might have unnecessary add/mult patterns now, but they also have the add/shift. I didn't see any big differences, and all are of the expected kind. Some targets need some love (but what else is new). The patch is approved for trunk. Thanks, Segher