| Message ID | 56422E54.6090404@samsung.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Nov 10, 2015 at 5:50 PM, Evandro Menezes <e.menezes@samsung.com> wrote: > 2015-11-10 Evandro Menezes <e.menezes@samsung.com> > > gcc/ > > * config/aarch64/aarch64.md (predicated): Copy attribute from > "arm.md". > > This patch duplicates an attribute from arm.md so that the same pipeline > model can be used for both AArch32 and AArch64. I'm not an aarch64 maintainer so I cannot approve. There are no predicated instructions in aarch64 - thus it's best imho to have only one option, "no" and not even give the option for someone to accidentally set this to yes. regards Ramana > > Bootstrapped on arm-unknown-linux-gnueabihf, aarch64-unknown-linux-gnu. > > Please, commit if it's alright. > > -- > Evandro Menezes > >
On Tue, Nov 10, 2015 at 6:01 PM, Ramana Radhakrishnan <ramana.gcc@googlemail.com> wrote: > On Tue, Nov 10, 2015 at 5:50 PM, Evandro Menezes <e.menezes@samsung.com> wrote: >> 2015-11-10 Evandro Menezes <e.menezes@samsung.com> >> >> gcc/ >> >> * config/aarch64/aarch64.md (predicated): Copy attribute from >> "arm.md". >> >> This patch duplicates an attribute from arm.md so that the same pipeline >> model can be used for both AArch32 and AArch64. > > I'm not an aarch64 maintainer so I cannot approve. > > There are no predicated instructions in aarch64 - thus it's best imho > to have only one option, "no" and not even give the option for someone > to accidentally set this to yes. Scratch that - I had a brain fade. Ramana > > regards > Ramana > > >> >> Bootstrapped on arm-unknown-linux-gnueabihf, aarch64-unknown-linux-gnu. >> >> Please, commit if it's alright. >> >> -- >> Evandro Menezes >> >>
On Tue, Nov 10, 2015 at 11:50:12AM -0600, Evandro Menezes wrote: > 2015-11-10 Evandro Menezes <e.menezes@samsung.com> > > gcc/ > > * config/aarch64/aarch64.md (predicated): Copy attribute from > "arm.md". > > This patch duplicates an attribute from arm.md so that the same > pipeline model can be used for both AArch32 and AArch64. > > Bootstrapped on arm-unknown-linux-gnueabihf, aarch64-unknown-linux-gnu. > > Please, commit if it's alright. > > -- > Evandro Menezes > > > From 3b643a3c026350864713e1700dc44e4794d93809 Mon Sep 17 00:00:00 2001 > From: Evandro Menezes <e.menezes@samsung.com> > Date: Mon, 9 Nov 2015 17:11:16 -0600 > Subject: [PATCH 1/2] [AArch64] Add attribute for compatibility with ARM > pipeline models > > gcc/ > * config/aarch64/aarch64.md (predicated): Copy attribute from "arm.md". > --- > gcc/config/aarch64/aarch64.md | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md > index 6b08850..2bc2ff5 100644 > --- a/gcc/config/aarch64/aarch64.md > +++ b/gcc/config/aarch64/aarch64.md > @@ -195,6 +195,11 @@ > ;; 1 :=: yes > (define_attr "far_branch" "" (const_int 0)) > > +;; [For compatibility with ARM in pipeline models] > +;; Attribute that specifies whether or not the instruction is executed > +;; conditionally (<C> != "AL"? "yes": "no"). I'm not sure this <C> != "AL" [...] part makes sense to me (thinking only of AArch64, I'd understand it on AArch32 :) ) and we should document that this is never set for AArch64. Could you respin with a slightly clearer comment. Thanks, James
On 11/12/2015 08:55 AM, James Greenhalgh wrote: > On Tue, Nov 10, 2015 at 11:50:12AM -0600, Evandro Menezes wrote: >> 2015-11-10 Evandro Menezes <e.menezes@samsung.com> >> >> gcc/ >> >> * config/aarch64/aarch64.md (predicated): Copy attribute from >> "arm.md". >> >> This patch duplicates an attribute from arm.md so that the same >> pipeline model can be used for both AArch32 and AArch64. >> >> Bootstrapped on arm-unknown-linux-gnueabihf, aarch64-unknown-linux-gnu. >> >> Please, commit if it's alright. >> >> -- >> Evandro Menezes >> >> >> From 3b643a3c026350864713e1700dc44e4794d93809 Mon Sep 17 00:00:00 2001 >> From: Evandro Menezes <e.menezes@samsung.com> >> Date: Mon, 9 Nov 2015 17:11:16 -0600 >> Subject: [PATCH 1/2] [AArch64] Add attribute for compatibility with ARM >> pipeline models >> >> gcc/ >> * config/aarch64/aarch64.md (predicated): Copy attribute from "arm.md". >> --- >> gcc/config/aarch64/aarch64.md | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md >> index 6b08850..2bc2ff5 100644 >> --- a/gcc/config/aarch64/aarch64.md >> +++ b/gcc/config/aarch64/aarch64.md >> @@ -195,6 +195,11 @@ >> ;; 1 :=: yes >> (define_attr "far_branch" "" (const_int 0)) >> >> +;; [For compatibility with ARM in pipeline models] >> +;; Attribute that specifies whether or not the instruction is executed >> +;; conditionally (<C> != "AL"? "yes": "no"). > I'm not sure this <C> != "AL" [...] part makes sense to me (thinking only > of AArch64, I'd understand it on AArch32 :) ) and we should document that > this is never set for AArch64. Could you respin with a slightly clearer > comment. Since this attribute was not described in config/arm/arm.md, I felt that it needed to, but perhaps in its original place instead. I agree that I should also point out that it's strictly for compatibility with AArch32 and that it never matters for AArch64. WRT the <C> thing, I was referring to the opcode fields terminology used by ARM in its ISA documentation. Perhaps it's unnecessary, yes? Thank you,
From 3b643a3c026350864713e1700dc44e4794d93809 Mon Sep 17 00:00:00 2001 From: Evandro Menezes <e.menezes@samsung.com> Date: Mon, 9 Nov 2015 17:11:16 -0600 Subject: [PATCH 1/2] [AArch64] Add attribute for compatibility with ARM pipeline models gcc/ * config/aarch64/aarch64.md (predicated): Copy attribute from "arm.md". --- gcc/config/aarch64/aarch64.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index 6b08850..2bc2ff5 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -195,6 +195,11 @@ ;; 1 :=: yes (define_attr "far_branch" "" (const_int 0)) +;; [For compatibility with ARM in pipeline models] +;; Attribute that specifies whether or not the instruction is executed +;; conditionally (<C> != "AL"? "yes": "no"). +(define_attr "predicated" "yes,no" (const_string "no")) + ;; ------------------------------------------------------------------- ;; Pipeline descriptions and scheduling ;; ------------------------------------------------------------------- -- 2.1.0.243.g30d45f7
2015-11-10 Evandro Menezes <e.menezes@samsung.com> gcc/ * config/aarch64/aarch64.md (predicated): Copy attribute from "arm.md". This patch duplicates an attribute from arm.md so that the same pipeline model can be used for both AArch32 and AArch64. Bootstrapped on arm-unknown-linux-gnueabihf, aarch64-unknown-linux-gnu. Please, commit if it's alright.