From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13495 invoked by alias); 16 May 2015 02:40:55 -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 13484 invoked by uid 89); 16 May 2015 02:40:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 X-HELO: arjuna.pair.com Received: from arjuna.pair.com (HELO arjuna.pair.com) (209.68.5.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Sat, 16 May 2015 02:40:52 +0000 Received: by arjuna.pair.com (Postfix, from userid 3006) id 5FC978A2BC; Fri, 15 May 2015 22:40:48 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by arjuna.pair.com (Postfix) with ESMTP id AB3868A29F; Fri, 15 May 2015 22:40:48 -0400 (EDT) Date: Sat, 16 May 2015 06:09:00 -0000 From: Hans-Peter Nilsson To: Segher Boessenkool cc: "Kumar, Venkataramanan" , "Jeff Law (law@redhat.com)" , "gcc-patches@gcc.gnu.org" , "maxim.kuvyrkov@linaro.org" Subject: Re: [RFC]: Remove Mem/address type assumption in combiner In-Reply-To: <20150501151758.GA1182@gate.crashing.org> Message-ID: References: <7794A52CE4D579448B959EED7DD0A4723DCE9F34@satlexdag06.amd.com> <20150429170335.GB21715@gate.crashing.org> <20150501151758.GA1182@gate.crashing.org> User-Agent: Alpine 2.02 (BSF 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-IsSubscribed: yes X-SW-Source: 2015-05/txt/msg01474.txt.bz2 On Fri, 1 May 2015, Segher Boessenkool wrote: > 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). Thanks for actually checking the impact. > Results are good, but > they show many targets need some work... > > > BIGGER > 2212322 2212706 cris Also observable as noted in PR66171, a regression: Running /tmp/hpautotest-gcc0/gcc/gcc/testsuite/gcc.target/cris/cris.exp ... FAIL: gcc.target/cris/biap.c scan-assembler addi FAIL: gcc.target/cris/biap.c scan-assembler-not lsl I confess the test-case-"guarded" addi pattern should have been expressed with a shift in addition to the multiplication. ("In addition to" as the canonically wrong one used to be the combine-matching pattern; I'm not sure I should really drop that just yet.) I'll have to check that expressing using a shift fixes this issue. Supposedly more noteworthy: this now-stricter canonicalization leads to a requirement to rewrite patterns that used to be: (parallel [(set reg0 (mem (plus (mult reg1 N) reg2))) (set reg3 (plus (mult reg1 N) reg2))]) into the awkwardly asymmetric: (parallel [(set reg0 (mem (plus (mult reg1 2) reg2))) (set reg3 (plus (ashift reg1 M) reg2))]) (where M = log2 N) and of course a need to verify that combine actually *does* make use of the pattern after the change at least as much as it did before. > Some targets need some love (but what else is new). Meh. Well, you now got some whine to go with that cheese. brgds, H-P