From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19226 invoked by alias); 25 Nov 2015 04:42:24 -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 19211 invoked by uid 89); 25 Nov 2015 04:42:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-vk0-f47.google.com Received: from mail-vk0-f47.google.com (HELO mail-vk0-f47.google.com) (209.85.213.47) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 25 Nov 2015 04:42:22 +0000 Received: by vkfr145 with SMTP id r145so27535834vkf.0 for ; Tue, 24 Nov 2015 20:42:20 -0800 (PST) MIME-Version: 1.0 X-Received: by 10.31.47.130 with SMTP id v124mr30892290vkv.117.1448426540150; Tue, 24 Nov 2015 20:42:20 -0800 (PST) Received: by 10.103.45.207 with HTTP; Tue, 24 Nov 2015 20:42:20 -0800 (PST) In-Reply-To: <5654343A.2080609@foss.arm.com> References: <000001d12119$49548570$dbfd9050$@arm.com> <20151117100800.GA6727@arm.com> <564F5ABF.2020302@foss.arm.com> <5654343A.2080609@foss.arm.com> Date: Wed, 25 Nov 2015 04:53:00 -0000 Message-ID: Subject: Re: [PATCH AArch64]Handle REG+REG+CONST and REG+NON_REG+CONST in legitimize address From: "Bin.Cheng" To: Richard Earnshaw Cc: James Greenhalgh , Bin Cheng , gcc-patches List , Vladimir Makarov , Andreas.Krebbel@de.ibm.com Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2015-11/txt/msg02977.txt.bz2 On Tue, Nov 24, 2015 at 5:56 PM, Richard Earnshaw wrote: > On 24/11/15 02:51, Bin.Cheng wrote: >>>> The aarch64's problem is we don't define addptr3 pattern, and we don't >>>> >> have direct insn pattern describing the "x + y << z". According to >>>> >> gcc internal: >>>> >> >>>> >> =E2=80=98addptrm3=E2=80=99 >>>> >> Like addm3 but is guaranteed to only be used for address calculatio= ns. >>>> >> The expanded code is not allowed to clobber the condition code. It >>>> >> only needs to be defined if addm3 sets the condition code. >>> > >>> > addm3 on aarch64 does not set the condition codes, so by this rule we >>> > shouldn't need to define this pattern. >> Hi Richard, >> I think that rule has a prerequisite that backend needs to support >> register shifted addition in addm3 pattern. > > addm3 is a named pattern and its format is well defined. It does not > take a shifted operand and never has. > >> Apparently for AArch64, >> addm3 only supports "reg+reg" or "reg+imm". Also we don't really >> "does not set the condition codes" actually, because both >> "adds_shift_imm_*" and "adds_mul_imm_*" do set the condition flags. > > You appear to be confusing named patterns (used by expand) with > recognizers. Anyway, we have I understand the difference between expanding and recognizer. I missed below non-set-condition-flags insn patterns as you pointed out, that's what I meant we don't support "does not set the condition codes" wrto either expanding or recognizer. Anyway, I was wrong and we do have such nameless patterns. > > (define_insn "*add__" > [(set (match_operand:GPI 0 "register_operand" "=3Dr") > (plus:GPI (ASHIFT:GPI (match_operand:GPI 1 "register_operand" "r") > (match_operand:QI 2 > "aarch64_shift_imm_" "n")) > (match_operand:GPI 3 "register_operand" "r")))] > > Which is a non-flag setting add with shifted operand. > >> Either way I think it is another backend issue, so do you approve that >> I commit this patch now? > > Not yet. I think there's something fundamental amiss here. > > BTW, it looks to me as though addptr3 should have exactly the same > operand rules as add3 (documentation reads "like add3"), so a > shifted operand shouldn't be supported there either. If that isn't the > case then that should be clearly called out in the documentation. The direct cause of this ICE is LRA generates below insn sequence when reloading "x=3Dy+z*const" where the mult part is actually a register scale operation: r1 =3D z*const x =3D y + r1 It generates mult directly but aarch64 doesn't have pattern describing mult with constant. We do have pattern for the corresponding shift insn. As a result, the constant mult insn is not recognized. This can be fixed by using force_operand to generate valid mult or shift instructions, is it feasible in lra? For this case, we can do better reload if we support scaled add in addptr pattern. After some archaeological work, I found addptr is added for s390 which supports addressing mode like "base + index + disp" and its variants. Maybe that's why the pattern's documentation doesn't explicitly describe the scaled operation. IMHO, this pattern should be target dependent and be able to handle addressing modes that each target supports. A side proof is in code and comment of function lra_emit_add. It explicitly handles, describes scaled add address expression. CCing Vlad and Andreas since they are the original authors and may help her= e. Thanks, bin