From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 100713 invoked by alias); 29 Apr 2015 19:22:45 -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 100701 invoked by uid 89); 29 Apr 2015 19:22:45 -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_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 29 Apr 2015 19:22:42 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t3TJMbxw005410 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 29 Apr 2015 15:22:37 -0400 Received: from localhost.localdomain (ovpn-113-143.phx2.redhat.com [10.3.113.143]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t3TJMbrX009937; Wed, 29 Apr 2015 15:22:37 -0400 Message-ID: <55412F7C.4020006@redhat.com> Date: Wed, 29 Apr 2015 19:31:00 -0000 From: Jeff Law User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: "Kumar, Venkataramanan" , "segher@kernel.crashing.org" , "gcc-patches@gcc.gnu.org" CC: "maxim.kuvyrkov@linaro.org" Subject: Re: [RFC]: Remove Mem/address type assumption in combiner References: <7794A52CE4D579448B959EED7DD0A4723DCE9F34@satlexdag06.amd.com> In-Reply-To: <7794A52CE4D579448B959EED7DD0A4723DCE9F34@satlexdag06.amd.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-04/txt/msg01923.txt.bz2 On 04/29/2015 03:25 AM, Kumar, Venkataramanan wrote: > Hi Jeff/Segher, > > When we see an RTX code with PLUS or MINUS then it is treated as MEM/address type (we are inside address RTX). > Is there any significance on that assumption? I removed this assumption and the test case in the PR 63949 passed. When appearing inside a MEM, we have different canonicalization rules. The comment in make_compound_operation clearly indicates that the PLUS/MINUS support is a hack. However, I'm pretty sure it was strictly for getting better code than for correctness. One path forward is to properly track if we're in a MEM, at which point the hack for PLUS/MINUS could probably just go away. > > 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. > > Tests that now fail, but worked before: > > gcc.target/aarch64/adds1.c scan-assembler adds\tw[0-9]+, w[0-9]+, w[0-9]+, lsl 3 > gcc.target/aarch64/adds1.c scan-assembler adds\tx[0-9]+, x[0-9]+, x[0-9]+, lsl 3 So that test seems to want to verify that you can generate a shift-add type instruction. I suspect the others are similar. I'd be curious to see the .combine dump as well as the final assembly for this test. Which is a strong hint that we should be looking at target with shift-add style instructions. ARM, AArch64, HPPA, x86 come immediately to mind. >> > 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. > > But before that I wanted to check if the assumption in combiner, can simply be removed ? Given what I'm seeing now, I doubt it can simply be removed at this point (which is a change in my position) since ports with these shift-add style instructions have probably been tuned to work with existing combine behaviour. We may need to do a deep test across various targets to identify those affected and fix them. jeff