From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29109 invoked by alias); 1 Dec 2015 03:19:15 -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 27538 invoked by uid 89); 1 Dec 2015 03:19:14 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,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; Tue, 01 Dec 2015 03:19:12 +0000 Received: by vkha189 with SMTP id a189so115344036vkh.2 for ; Mon, 30 Nov 2015 19:19:10 -0800 (PST) MIME-Version: 1.0 X-Received: by 10.31.48.149 with SMTP id w143mr57388480vkw.5.1448939950347; Mon, 30 Nov 2015 19:19:10 -0800 (PST) Received: by 10.103.45.207 with HTTP; Mon, 30 Nov 2015 19:19:10 -0800 (PST) In-Reply-To: <56543963.3070704@foss.arm.com> References: <000001d12119$49548570$dbfd9050$@arm.com> <20151117100800.GA6727@arm.com> <564F5ABF.2020302@foss.arm.com> <5654343A.2080609@foss.arm.com> <56543963.3070704@foss.arm.com> Date: Tue, 01 Dec 2015 03:19: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 Content-Type: multipart/mixed; boundary=001a1143e8200ac6d30525cd9f44 X-IsSubscribed: yes X-SW-Source: 2015-12/txt/msg00001.txt.bz2 --001a1143e8200ac6d30525cd9f44 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Content-length: 3798 On Tue, Nov 24, 2015 at 6:18 PM, Richard Earnshaw wrote: > On 24/11/15 09:56, 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 >> >> (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. >> >> R. >> > > PS. > > I presume you are aware of the canonicalization rules for add? That is, > for a shift-and-add operation, the shift operand must appear first. Ie. > > (plus (shift (op, op)), op) > > not > > (plus (op, (shift (op, op)) Hi Richard, Thanks for the comments. I realized that the not-recognized insn issue is because the original patch build non-canonical expressions. When reloading address expression, LRA generates non-canonical register scaled insn, which can't be recognized by aarch64 backend. Here is the updated patch using canonical form pattern, it passes bootstrap and regression test. Well, the ivo failure still exists, but it analyzed in the original message. Is this patch OK? As for Jiong's concern about the additional extension instruction, I think this only stands for atmoic load store instructions. For general load store, AArch64 supports zext/sext in register scaling addressing mode, the additional instruction can be forward propagated into memory reference. The problem for atomic load store is AArch64 only supports direct register addressing mode. After LRA reloads address expression out of memory reference, there is no combine/fwprop optimizer to merge instructions. The problem is atomic_store's predicate doesn't match its constraint. The predicate used for atomic_store is memory_operand, while all other atomic patterns use aarch64_sync_memory_operand. I think this might be a typo. With this change, expand will not generate addressing mode requiring reload anymore. I will test another patch fixing this. Thanks, bin > > R. --001a1143e8200ac6d30525cd9f44 Content-Type: text/plain; charset=US-ASCII; name="aarch64_legitimize_addr-20151128.txt" Content-Disposition: attachment; filename="aarch64_legitimize_addr-20151128.txt" Content-Transfer-Encoding: base64 X-Attachment-Id: f_ihmtfb480 Content-length: 3477 ZGlmZiAtLWdpdCBhL2djYy9jb25maWcvYWFyY2g2NC9hYXJjaDY0LmMgYi9n Y2MvY29uZmlnL2FhcmNoNjQvYWFyY2g2NC5jCmluZGV4IDNmZTJmMGYuLjVi M2UzYzQgMTAwNjQ0Ci0tLSBhL2djYy9jb25maWcvYWFyY2g2NC9hYXJjaDY0 LmMKKysrIGIvZ2NjL2NvbmZpZy9hYXJjaDY0L2FhcmNoNjQuYwpAQCAtNDc1 NywxMyArNDc1Nyw2NSBAQCBhYXJjaDY0X2xlZ2l0aW1pemVfYWRkcmVzcyAo cnR4IHgsIHJ0eCAvKiBvcmlnX3ggICovLCBtYWNoaW5lX21vZGUgbW9kZSkK ICAgICAgV2UgdHJ5IHRvIHBpY2sgYXMgbGFyZ2UgYSByYW5nZSBmb3IgdGhl IG9mZnNldCBhcyBwb3NzaWJsZSB0bwogICAgICBtYXhpbWl6ZSB0aGUgY2hh bmNlIG9mIGEgQ1NFLiAgSG93ZXZlciwgZm9yIGFsaWduZWQgYWRkcmVzc2Vz CiAgICAgIHdlIGxpbWl0IHRoZSByYW5nZSB0byA0ayBzbyB0aGF0IHN0cnVj dHVyZXMgd2l0aCBkaWZmZXJlbnQgc2l6ZWQKLSAgICAgZWxlbWVudHMgYXJl IGxpa2VseSB0byB1c2UgdGhlIHNhbWUgYmFzZS4gICovCisgICAgIGVsZW1l bnRzIGFyZSBsaWtlbHkgdG8gdXNlIHRoZSBzYW1lIGJhc2UuICBXZSBuZWVk IHRvIGJlIGNhcmVmdWwKKyAgICAgbm90IHNwbGl0IENPTlNUIGZvciBzb21l IGZvcm1zIGFkZHJlc3MgZXhwcmVzc2lvbnMsIG90aGVyd2lzZSBpdAorICAg ICB3aWxsIGdlbmVyYXRlIHN1Yi1vcHRpbWFsIGNvZGUuICAqLwogCiAgIGlm IChHRVRfQ09ERSAoeCkgPT0gUExVUyAmJiBDT05TVF9JTlRfUCAoWEVYUCAo eCwgMSkpKQogICAgIHsKICAgICAgIEhPU1RfV0lERV9JTlQgb2Zmc2V0ID0g SU5UVkFMIChYRVhQICh4LCAxKSk7CiAgICAgICBIT1NUX1dJREVfSU5UIGJh c2Vfb2Zmc2V0OwogCisgICAgICBpZiAoR0VUX0NPREUgKFhFWFAgKHgsIDAp KSA9PSBQTFVTKQorCXsKKwkgIHJ0eCBvcDAgPSBYRVhQIChYRVhQICh4LCAw KSwgMCk7CisJICBydHggb3AxID0gWEVYUCAoWEVYUCAoeCwgMCksIDEpOwor CisJICAvKiBGb3IgYWRkciBleHByZXNzaW9uIGluIHRoZSBmb3JtIGxpa2Ug InIxICsgcjIgKyAweDNmZmMiLgorCSAgICAgU2luY2UgdGhlIG9mZnNldCBp cyB3aXRoaW4gcmFuZ2Ugc3VwcG9ydGVkIGJ5IGFkZHJlc3NpbmcKKwkgICAg IG1vZGUgInJlZytvZmZzZXQiLCB3ZSBkb24ndCBzcGxpdCB0aGUgY29uc3Qg YW5kIGxlZ2FsaXplCisJICAgICBpdCBpbnRvIGJlbG93IGluc24gYW5kIGV4 cHIgc2VxdWVuY2U6CisJICAgICAgIHIzID0gcjEgKyByMjsKKwkgICAgICAg InIzICsgMHgzZmZjIi4gICovCisJICBpZiAoUkVHX1AgKG9wMCkgJiYgUkVH X1AgKG9wMSkpCisJICAgIHsKKwkgICAgICBtYWNoaW5lX21vZGUgYWRkcl9t b2RlID0gR0VUX01PREUgKHgpOworCSAgICAgIHJ0eCBiYXNlID0gZ2VuX3Jl Z19ydHggKGFkZHJfbW9kZSk7CisJICAgICAgcnR4IGFkZHIgPSBwbHVzX2Nv bnN0YW50IChhZGRyX21vZGUsIGJhc2UsIG9mZnNldCk7CisKKwkgICAgICBp ZiAoYWFyY2g2NF9sZWdpdGltYXRlX2FkZHJlc3NfaG9va19wIChtb2RlLCBh ZGRyLCBmYWxzZSkpCisJCXsKKwkJICBlbWl0X2luc24gKGdlbl9hZGRkaTMg KGJhc2UsIG9wMCwgb3AxKSk7CisJCSAgcmV0dXJuIGFkZHI7CisJCX0KKwkg ICAgfQorCSAgLyogRm9yIGFkZHIgZXhwcmVzc2lvbiBpbiB0aGUgZm9ybSBs aWtlICJyMSArIHIyPDwyICsgMHgzZmZjIi4KKwkgICAgIExpdmUgYWJvdmUs IHdlIGRvbid0IHNwbGl0IHRoZSBjb25zdCBhbmQgbGVnYWxpemUgaXQgaW50 bworCSAgICAgYmVsb3cgaW5zbiBhbmQgZXhwciBzZXF1ZW5jZToKKwkgICAg ICAgcjMgPSAweDNmZmM7CisJICAgICAgIHI0ID0gcjEgKyByMzsKKwkgICAg ICAgInI0ICsgcjI8PDIiLiAgKi8KKwkgIGVsc2UgaWYgKFJFR19QIChvcDAp IHx8IFJFR19QIChvcDEpKQorCSAgICB7CisJICAgICAgbWFjaGluZV9tb2Rl IGFkZHJfbW9kZSA9IEdFVF9NT0RFICh4KTsKKwkgICAgICBydHggYmFzZSA9 IGdlbl9yZWdfcnR4IChhZGRyX21vZGUpOworCisJICAgICAgLyogU3dpdGNo IHRvIG1ha2Ugc3VyZSB0aGF0IHJlZ2lzdGVyIGlzIGluIG9wMC4gICovCisJ ICAgICAgaWYgKFJFR19QIChvcDEpKQorCQlzdGQ6OnN3YXAgKG9wMCwgb3Ax KTsKKworCSAgICAgIHJ0eCBhZGRyID0gZ2VuX3J0eF9mbXRfZWUgKFBMVVMs IGFkZHJfbW9kZSwgb3AxLCBiYXNlKTsKKworCSAgICAgIGlmIChhYXJjaDY0 X2xlZ2l0aW1hdGVfYWRkcmVzc19ob29rX3AgKG1vZGUsIGFkZHIsIGZhbHNl KSkKKwkJeworCQkgIGJhc2UgPSBmb3JjZV9vcGVyYW5kIChwbHVzX2NvbnN0 YW50IChhZGRyX21vZGUsCisJCQkJCQkgICAgICAgb3AwLCBvZmZzZXQpLAor CQkJCQlOVUxMX1JUWCk7CisJCSAgcmV0dXJuIGdlbl9ydHhfZm10X2VlIChQ TFVTLCBhZGRyX21vZGUsIG9wMSwgYmFzZSk7CisJCX0KKwkgICAgfQorCX0K KwogICAgICAgLyogRG9lcyBpdCBsb29rIGxpa2Ugd2UnbGwgbmVlZCBhIGxv YWQvc3RvcmUtcGFpciBvcGVyYXRpb24/ICAqLwogICAgICAgaWYgKEdFVF9N T0RFX1NJWkUgKG1vZGUpID4gMTYKIAkgIHx8IG1vZGUgPT0gVEltb2RlKQo= --001a1143e8200ac6d30525cd9f44--