[PATCH] PowerPC: Add 'prefix' to the 'isa' attribute

[PATCH] PowerPC: Add 'prefix' to the 'isa' attribute

[Michael Meissner]

Some of my future patches for prefixed memory instructions in a future PowerPC
processor need the following tweak to the ISA attribute, so that the
alternative that uses a prefixed instruction to load up large integer constants
can be eliminated if the user does not compile for the 'future' target.

I have bootstrapped the compiler and run make check with no regressions.  Can I
check this into the trunk?

2019-06-17  Michael Meissner  <meissner@linux.ibm.com>

	* config/rs6000/rs6000.md (isa attribute): Add support for
	prefixed instructions.

Index: gcc/config/rs6000/rs6000.md
===================================================================
--- gcc/config/rs6000/rs6000.md	(revision 272270)
+++ gcc/config/rs6000/rs6000.md	(working copy)
@@ -267,7 +267,9 @@ (define_attr "cpu"
   (const (symbol_ref "(enum attr_cpu) rs6000_tune")))
 
 ;; The ISA we implement.
-(define_attr "isa" "any,p5,p6,p7,p7v,p8v,p9v,p9kf,p9tf" (const_string "any"))
+(define_attr "isa"
+  "any,p5,p6,p7,p7v,p8v,p9v,p9kf,p9tf,prefix"
+  (const_string "any"))
 
 ;; Is this alternative enabled for the current CPU/ISA/etc.?
 (define_attr "enabled" ""
@@ -306,6 +308,10 @@ (define_attr "enabled" ""
      (and (eq_attr "isa" "p9tf")
 	  (match_test "FLOAT128_VECTOR_P (TFmode)"))
      (const_int 1)
+
+     (and (eq_attr "isa" "prefix")
+	  (match_test "TARGET_PREFIXED_ADDR"))
+     (const_int 1)
     ] (const_int 0)))
 
 ;; If this instruction is microcoded on the CELL processor

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.ibm.com, phone: +1 (978) 899-4797

Re: [PATCH] PowerPC: Add 'prefix' to the 'isa' attribute

[Segher Boessenkool]

On Mon, Jun 17, 2019 at 08:04:42PM -0400, Michael Meissner wrote:
> --- gcc/config/rs6000/rs6000.md	(revision 272270)
> +++ gcc/config/rs6000/rs6000.md	(working copy)
> @@ -267,7 +267,9 @@ (define_attr "cpu"
>    (const (symbol_ref "(enum attr_cpu) rs6000_tune")))
>  
>  ;; The ISA we implement.
> -(define_attr "isa" "any,p5,p6,p7,p7v,p8v,p9v,p9kf,p9tf" (const_string "any"))
> +(define_attr "isa"
> +  "any,p5,p6,p7,p7v,p8v,p9v,p9kf,p9tf,prefix"
> +  (const_string "any"))

Don't break the line unnecessarily.

"prefix" is not a good name.  Maybe use "futp" now, so it is in line with
the other isa values?  I.e. the "prefix" part of the "future" isa.  Or
"futx"?  Short names are important, keep it to 4 chars?

(The isa values are named after the processor that first implements them,
not the ISA versions, because 2.07 is much easier to get wrong than p8 is).


Segher

[PATCH, v2] PowerPC: Add 'prefix' to the 'isa' attribute

[Michael Meissner]

On Tue, Jun 18, 2019 at 07:13:22AM -0500, Segher Boessenkool wrote:
> On Mon, Jun 17, 2019 at 08:04:42PM -0400, Michael Meissner wrote:
> > --- gcc/config/rs6000/rs6000.md	(revision 272270)
> > +++ gcc/config/rs6000/rs6000.md	(working copy)
> > @@ -267,7 +267,9 @@ (define_attr "cpu"
> >    (const (symbol_ref "(enum attr_cpu) rs6000_tune")))
> >  
> >  ;; The ISA we implement.
> > -(define_attr "isa" "any,p5,p6,p7,p7v,p8v,p9v,p9kf,p9tf" (const_string "any"))
> > +(define_attr "isa"
> > +  "any,p5,p6,p7,p7v,p8v,p9v,p9kf,p9tf,prefix"
> > +  (const_string "any"))
> 
> Don't break the line unnecessarily.
> 
> "prefix" is not a good name.  Maybe use "futp" now, so it is in line with
> the other isa values?  I.e. the "prefix" part of the "future" isa.  Or
> "futx"?  Short names are important, keep it to 4 chars?
> 
> (The isa values are named after the processor that first implements them,
> not the ISA versions, because 2.07 is much easier to get wrong than p8 is).

Here is version 2:

2019-06-19  Michael Meissner  <meissner@linux.ibm.com>

	* config/rs6000/rs6000.md (isa attribute): Add support for
	for a future processor.

Index: gcc/config/rs6000/rs6000.md
===================================================================
--- gcc/config/rs6000/rs6000.md	(revision 272439)
+++ gcc/config/rs6000/rs6000.md	(working copy)
@@ -267,7 +267,8 @@ (define_attr "cpu"
   (const (symbol_ref "(enum attr_cpu) rs6000_tune")))
 
 ;; The ISA we implement.
-(define_attr "isa" "any,p5,p6,p7,p7v,p8v,p9v,p9kf,p9tf" (const_string "any"))
+(define_attr "isa" "any,p5,p6,p7,p7v,p8v,p9v,p9kf,p9tf,futp"
+  (const_string "any"))
 
 ;; Is this alternative enabled for the current CPU/ISA/etc.?
 (define_attr "enabled" ""
@@ -306,6 +307,10 @@ (define_attr "enabled" ""
      (and (eq_attr "isa" "p9tf")
 	  (match_test "FLOAT128_VECTOR_P (TFmode)"))
      (const_int 1)
+
+     (and (eq_attr "isa" "futp")
+	  (match_test "TARGET_FUTURE"))
+     (const_int 1)
     ] (const_int 0)))
 
 ;; If this instruction is microcoded on the CELL processor

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.ibm.com, phone: +1 (978) 899-4797

Re: [PATCH, v2] PowerPC: Add 'prefix' to the 'isa' attribute

[Segher Boessenkool]

Hi Mike,

On Wed, Jun 19, 2019 at 10:43:43AM -0400, Michael Meissner wrote:
> Here is version 2:
> 
> 2019-06-19  Michael Meissner  <meissner@linux.ibm.com>
> 
> 	* config/rs6000/rs6000.md (isa attribute): Add support for
> 	for a future processor.
> 
> Index: gcc/config/rs6000/rs6000.md
> ===================================================================
> --- gcc/config/rs6000/rs6000.md	(revision 272439)
> +++ gcc/config/rs6000/rs6000.md	(working copy)
> @@ -267,7 +267,8 @@ (define_attr "cpu"
>    (const (symbol_ref "(enum attr_cpu) rs6000_tune")))
>  
>  ;; The ISA we implement.
> -(define_attr "isa" "any,p5,p6,p7,p7v,p8v,p9v,p9kf,p9tf" (const_string "any"))
> +(define_attr "isa" "any,p5,p6,p7,p7v,p8v,p9v,p9kf,p9tf,futp"
> +  (const_string "any"))

Note to self: I should add a comment explaining what the values here mean.

> @@ -306,6 +307,10 @@ (define_attr "enabled" ""
>       (and (eq_attr "isa" "p9tf")
>  	  (match_test "FLOAT128_VECTOR_P (TFmode)"))
>       (const_int 1)
> +
> +     (and (eq_attr "isa" "futp")
> +	  (match_test "TARGET_FUTURE"))
> +     (const_int 1)
>      ] (const_int 0)))

Hrm, so maybe this should just be called "fut"?  (We don't yet have values
for just "p8" and "p9", nothing uses those yet, but of course we should
have those too, eventually.)

Okay for trunk with that maybe fixed.  Thanks!


Segher