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

[Evandro Menezes <e dot menezes at samsung dot com>]

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 <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?

   2015-11-12  Evandro Menezes <e.menezes@samsung.com>

   [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 <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".
	* 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