Re: [PATCH 3a/4][AArch64] Add attribute for compatibility with ARM pipeline models

2015-11-20 Thread Kyrill Tkachov 


On 20/11/15 12:27, James Greenhalgh wrote:

On Thu, Nov 12, 2015 at 11:32:36AM -0600, Evandro Menezes wrote:

On 11/12/2015 09:39 AM, Evandro Menezes wrote:
2015-11-12  Evandro Menezes 

[AArch64] Add attribute for compatibility with ARM pipeline models

gcc/

* config/aarch64/aarch64.md (predicated): Copy attribute from
"arm.md".
* config/arm/arm.md (predicated): Added description.


The arm part is ok too. It's just a comment.
In the ChangeLog entry for arm.md I'd say "Add description."

Thanks,
Kyrill


Please, commit if it's alright.

The AArch64 part of this is OK.


 From 3fa6a2bca8f3d2992b4607cff0afcc2d9caa96f4 Mon Sep 17 00:00:00 2001
From: Evandro Menezes 
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".
* config/arm/arm.md (predicated): Added description.
---
  gcc/config/aarch64/aarch64.md | 4 
  gcc/config/arm/arm.md | 3 +++
  2 files changed, 7 insertions(+)

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 1586256..d46f837 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -195,6 +195,10 @@
  ;; 1 :=: yes
  (define_attr "far_branch" "" (const_int 0))
  
+;; Strictly for compatibility with AArch32 in pipeline models, since AArch64 has

+;; no predicated insns.
+(define_attr "predicated" "yes,no" (const_string "no"))
+
  ;; ---
  ;; Pipeline descriptions and scheduling
  ;; ---
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 73c3088..6bda491 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -105,6 +105,9 @@
  (define_attr "fpu" "none,vfp"
(const (symbol_ref "arm_fpu_attr")))
  
+; Predicated means that the insn form is conditionally executed based on a

+; predicate.  We default to 'no' because no Thumb patterns match this rule
+; and not all ARM insns do.

s/is conditionally executed/can be conditionally executed/ in the first
sentence. Otherwise, this looks OK to me but I can't approve the ARM part,
so you'll need to wait for a review from someone who can.

Thanks,
James


  (define_attr "predicated" "yes,no" (const_string "no"))
  
  ; LENGTH of an instruction (in bytes)

--
2.1.0.243.g30d45f7

Re: [PATCH 3a/4][AArch64] Add attribute for compatibility with ARM pipeline models

2015-11-20 Thread James Greenhalgh 
On Thu, Nov 12, 2015 at 11:32:36AM -0600, Evandro Menezes wrote:
> On 11/12/2015 09:39 AM, Evandro Menezes wrote:
>2015-11-12  Evandro Menezes 
> 
>[AArch64] Add attribute for compatibility with ARM pipeline models
> 
>gcc/
> 
>* config/aarch64/aarch64.md (predicated): Copy attribute from
>"arm.md".
>* config/arm/arm.md (predicated): Added description.
> 
> Please, commit if it's alright.

The AArch64 part of this is OK.

> From 3fa6a2bca8f3d2992b4607cff0afcc2d9caa96f4 Mon Sep 17 00:00:00 2001
> From: Evandro Menezes 
> 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".
>   * config/arm/arm.md (predicated): Added description.
> ---
>  gcc/config/aarch64/aarch64.md | 4 
>  gcc/config/arm/arm.md | 3 +++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 1586256..d46f837 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -195,6 +195,10 @@
>  ;; 1 :=: yes
>  (define_attr "far_branch" "" (const_int 0))
>  
> +;; Strictly for compatibility with AArch32 in pipeline models, since AArch64 
> has
> +;; no predicated insns.
> +(define_attr "predicated" "yes,no" (const_string "no"))
> +
>  ;; ---
>  ;; Pipeline descriptions and scheduling
>  ;; ---
> diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
> index 73c3088..6bda491 100644
> --- a/gcc/config/arm/arm.md
> +++ b/gcc/config/arm/arm.md
> @@ -105,6 +105,9 @@
>  (define_attr "fpu" "none,vfp"
>(const (symbol_ref "arm_fpu_attr")))
>  
> +; Predicated means that the insn form is conditionally executed based on a
> +; predicate.  We default to 'no' because no Thumb patterns match this rule
> +; and not all ARM insns do.

s/is conditionally executed/can be conditionally executed/ in the first
sentence. Otherwise, this looks OK to me but I can't approve the ARM part,
so you'll need to wait for a review from someone who can.

Thanks,
James

>  (define_attr "predicated" "yes,no" (const_string "no"))
>  
>  ; LENGTH of an instruction (in bytes)
> -- 
> 2.1.0.243.g30d45f7
>

Re: [PATCH 3a/4][AArch64] Add attribute for compatibility with ARM pipeline models

2015-11-20 Thread Evandro Menezes 

On 11/20/2015 08:34 AM, Kyrill Tkachov wrote:


On 20/11/15 12:27, James Greenhalgh wrote:

On Thu, Nov 12, 2015 at 11:32:36AM -0600, Evandro Menezes wrote:

On 11/12/2015 09:39 AM, Evandro Menezes wrote:
2015-11-12  Evandro Menezes 

[AArch64] Add attribute for compatibility with ARM pipeline models

gcc/

* config/aarch64/aarch64.md (predicated): Copy attribute from
"arm.md".
* config/arm/arm.md (predicated): Added description.


The arm part is ok too. It's just a comment.
In the ChangeLog entry for arm.md I'd say "Add description."

Thanks,
Kyrill


Please, commit if it's alright.

The AArch64 part of this is OK.


 From 3fa6a2bca8f3d2992b4607cff0afcc2d9caa96f4 Mon Sep 17 00:00:00 2001
From: Evandro Menezes 
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".

* config/arm/arm.md (predicated): Added description.
---
  gcc/config/aarch64/aarch64.md | 4 
  gcc/config/arm/arm.md | 3 +++
  2 files changed, 7 insertions(+)

diff --git a/gcc/config/aarch64/aarch64.md 
b/gcc/config/aarch64/aarch64.md

index 1586256..d46f837 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -195,6 +195,10 @@
  ;; 1 :=: yes
  (define_attr "far_branch" "" (const_int 0))
  +;; Strictly for compatibility with AArch32 in pipeline models, 
since AArch64 has

+;; no predicated insns.
+(define_attr "predicated" "yes,no" (const_string "no"))
+
  ;; 
---

  ;; Pipeline descriptions and scheduling
  ;; 
---

diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 73c3088..6bda491 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -105,6 +105,9 @@
  (define_attr "fpu" "none,vfp"
(const (symbol_ref "arm_fpu_attr")))
  +; Predicated means that the insn form is conditionally executed 
based on a
+; predicate.  We default to 'no' because no Thumb patterns match 
this rule

+; and not all ARM insns do.

s/is conditionally executed/can be conditionally executed/ in the first
sentence. Otherwise, this looks OK to me but I can't approve the ARM 
part,

so you'll need to wait for a review from someone who can.

Thanks,
James


  (define_attr "predicated" "yes,no" (const_string "no"))
; LENGTH of an instruction (in bytes)
--
2.1.0.243.g30d45f7


Can you please commit it with this change in the Changelog?

Thank you,

--
Evandro Menezes

Re: [PATCH 3a/4][AArch64] Add attribute for compatibility with ARM pipeline models

2015-11-20 Thread Evandro Menezes 

On 11/20/2015 06:27 AM, James Greenhalgh wrote:

On Thu, Nov 12, 2015 at 11:32:36AM -0600, Evandro Menezes wrote:

On 11/12/2015 09:39 AM, Evandro Menezes wrote:
2015-11-12  Evandro Menezes 

[AArch64] Add attribute for compatibility with ARM pipeline models

gcc/

* config/aarch64/aarch64.md (predicated): Copy attribute from
"arm.md".
* config/arm/arm.md (predicated): Added description.

Please, commit if it's alright.

The AArch64 part of this is OK.


 From 3fa6a2bca8f3d2992b4607cff0afcc2d9caa96f4 Mon Sep 17 00:00:00 2001
From: Evandro Menezes 
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".
* config/arm/arm.md (predicated): Added description.
---
  gcc/config/aarch64/aarch64.md | 4 
  gcc/config/arm/arm.md | 3 +++
  2 files changed, 7 insertions(+)

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 1586256..d46f837 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -195,6 +195,10 @@
  ;; 1 :=: yes
  (define_attr "far_branch" "" (const_int 0))
  
+;; Strictly for compatibility with AArch32 in pipeline models, since AArch64 has

+;; no predicated insns.
+(define_attr "predicated" "yes,no" (const_string "no"))
+
  ;; ---
  ;; Pipeline descriptions and scheduling
  ;; ---
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 73c3088..6bda491 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -105,6 +105,9 @@
  (define_attr "fpu" "none,vfp"
(const (symbol_ref "arm_fpu_attr")))
  
+; Predicated means that the insn form is conditionally executed based on a

+; predicate.  We default to 'no' because no Thumb patterns match this rule
+; and not all ARM insns do.

s/is conditionally executed/can be conditionally executed/ in the first
sentence. Otherwise, this looks OK to me but I can't approve the ARM part,
so you'll need to wait for a review from someone who can.


  (define_attr "predicated" "yes,no" (const_string "no"))
  
  ; LENGTH of an instruction (in bytes)

--
2.1.0.243.g30d45f7


Actually, that would then be the existing description for the 
"predicable" attribute.  I think that this proposed description is 
correct for the "predicated" attribute.


Thank you,

--
Evandro Menezes

Re: [PATCH 3a/4][AArch64] Add attribute for compatibility with ARM pipeline models

2015-11-20 Thread James Greenhalgh 
On Fri, Nov 20, 2015 at 09:55:29AM -0600, Evandro Menezes wrote:
> On 11/20/2015 06:27 AM, James Greenhalgh wrote:
> >On Thu, Nov 12, 2015 at 11:32:36AM -0600, Evandro Menezes wrote:
> >>On 11/12/2015 09:39 AM, Evandro Menezes wrote:
> >>2015-11-12  Evandro Menezes 
> >>
> >>[AArch64] Add attribute for compatibility with ARM pipeline models
> >>
> >>gcc/
> >>
> >>* config/aarch64/aarch64.md (predicated): Copy attribute from
> >>"arm.md".
> >>* config/arm/arm.md (predicated): Added description.
> >>
> >>Please, commit if it's alright.
> >The AArch64 part of this is OK.
> >
> >> From 3fa6a2bca8f3d2992b4607cff0afcc2d9caa96f4 Mon Sep 17 00:00:00 2001
> >>From: Evandro Menezes 
> >>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".
> >>* config/arm/arm.md (predicated): Added description.
> >>---
> >>  gcc/config/aarch64/aarch64.md | 4 
> >>  gcc/config/arm/arm.md | 3 +++
> >>  2 files changed, 7 insertions(+)
> >>
> >>diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> >>index 1586256..d46f837 100644
> >>--- a/gcc/config/aarch64/aarch64.md
> >>+++ b/gcc/config/aarch64/aarch64.md
> >>@@ -195,6 +195,10 @@
> >>  ;; 1 :=: yes
> >>  (define_attr "far_branch" "" (const_int 0))
> >>+;; Strictly for compatibility with AArch32 in pipeline models, since 
> >>AArch64 has
> >>+;; no predicated insns.
> >>+(define_attr "predicated" "yes,no" (const_string "no"))
> >>+
> >>  ;; ---
> >>  ;; Pipeline descriptions and scheduling
> >>  ;; ---
> >>diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
> >>index 73c3088..6bda491 100644
> >>--- a/gcc/config/arm/arm.md
> >>+++ b/gcc/config/arm/arm.md
> >>@@ -105,6 +105,9 @@
> >>  (define_attr "fpu" "none,vfp"
> >>(const (symbol_ref "arm_fpu_attr")))
> >>+; Predicated means that the insn form is conditionally executed based on a
> >>+; predicate.  We default to 'no' because no Thumb patterns match this rule
> >>+; and not all ARM insns do.
> >s/is conditionally executed/can be conditionally executed/ in the first
> >sentence. Otherwise, this looks OK to me but I can't approve the ARM part,
> >so you'll need to wait for a review from someone who can.
> >
> >>  (define_attr "predicated" "yes,no" (const_string "no"))
> >>  ; LENGTH of an instruction (in bytes)
> >>-- 
> >>2.1.0.243.g30d45f7
> 
> Actually, that would then be the existing description for the
> "predicable" attribute.  I think that this proposed description is
> correct for the "predicated" attribute.

Right, understood, that will teach me to pattern match up to pred and stop
reading the word.

This is fine, I've committed it on your behalf as r230666

Thanks,
James

Re: [PATCH 3a/4][AArch64] Add attribute for compatibility with ARM pipeline models

2015-11-19 Thread Evandro Menezes 

On 11/12/2015 11:32 AM, Evandro Menezes wrote:

On 11/12/2015 09:39 AM, Evandro Menezes wrote:

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 

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 
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 ( != "AL"? "yes": "no").
I'm not sure this  != "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  thing, I was referring to the opcode fields terminology 
used by ARM in its ISA documentation.  Perhaps it's unnecessary, yes?




   2015-11-12  Evandro Menezes 

   [AArch64] Add attribute for compatibility with ARM pipeline models

   gcc/

   * config/aarch64/aarch64.md (predicated): Copy attribute from
   "arm.md".
   * config/arm/arm.md (predicated): Added description.

Please, commit if it's alright.


Ping.

--
Evandro Menezes

Re: [PATCH 3a/4][AArch64] Add attribute for compatibility with ARM pipeline models

2015-11-12 Thread Evandro Menezes 

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 

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 
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 ( != "AL"? "yes": "no").

I'm not sure this  != "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  thing, I was referring to the opcode fields terminology used 
by ARM in its ISA documentation.  Perhaps it's unnecessary, yes?


Thank you,

--
Evandro Menezes

Re: [PATCH 3a/4][AArch64] Add attribute for compatibility with ARM pipeline models

2015-11-12 Thread James Greenhalgh 
On Tue, Nov 10, 2015 at 11:50:12AM -0600, Evandro Menezes wrote:
>2015-11-10  Evandro Menezes 
> 
>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 
> 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 ( != "AL"? "yes": "no").

I'm not sure this  != "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

Re: [PATCH 3a/4][AArch64] Add attribute for compatibility with ARM pipeline models

2015-11-12 Thread Evandro Menezes 

On 11/12/2015 09:39 AM, Evandro Menezes wrote:

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 

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 
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 ( != "AL"? "yes": "no").
I'm not sure this  != "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  thing, I was referring to the opcode fields terminology 
used by ARM in its ISA documentation.  Perhaps it's unnecessary, yes?




   2015-11-12  Evandro Menezes 

   [AArch64] Add attribute for compatibility with ARM pipeline models

   gcc/

   * config/aarch64/aarch64.md (predicated): Copy attribute from
   "arm.md".
   * config/arm/arm.md (predicated): Added description.

Please, commit if it's alright.

--
Evandro Menezes

>From 3fa6a2bca8f3d2992b4607cff0afcc2d9caa96f4 Mon Sep 17 00:00:00 2001
From: Evandro Menezes 
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".
	* config/arm/arm.md (predicated): Added description.
---
 gcc/config/aarch64/aarch64.md | 4 
 gcc/config/arm/arm.md | 3 +++
 2 files changed, 7 insertions(+)

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 1586256..d46f837 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -195,6 +195,10 @@
 ;; 1 :=: yes
 (define_attr "far_branch" "" (const_int 0))
 
+;; Strictly for compatibility with AArch32 in pipeline models, since AArch64 has
+;; no predicated insns.
+(define_attr "predicated" "yes,no" (const_string "no"))
+
 ;; ---
 ;; Pipeline descriptions and scheduling
 ;; ---
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 73c3088..6bda491 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -105,6 +105,9 @@
 (define_attr "fpu" "none,vfp"
   (const (symbol_ref "arm_fpu_attr")))
 
+; Predicated means that the insn form is conditionally executed based on a
+; predicate.  We default to 'no' because no Thumb patterns match this rule
+; and not all ARM insns do.
 (define_attr "predicated" "yes,no" (const_string "no"))
 
 ; LENGTH of an instruction (in bytes)
-- 
2.1.0.243.g30d45f7

Re: [PATCH 3a/4][AArch64] Add attribute for compatibility with ARM pipeline models

2015-11-10 Thread Ramana Radhakrishnan 
On Tue, Nov 10, 2015 at 6:01 PM, Ramana Radhakrishnan
 wrote:
> On Tue, Nov 10, 2015 at 5:50 PM, Evandro Menezes  
> wrote:
>>2015-11-10  Evandro Menezes 
>>
>>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
>>
>>

Re: [PATCH 3a/4][AArch64] Add attribute for compatibility with ARM pipeline models

2015-11-10 Thread Ramana Radhakrishnan 
On Tue, Nov 10, 2015 at 5:50 PM, Evandro Menezes  wrote:
>2015-11-10  Evandro Menezes 
>
>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
>
>