public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH, rs6000] Cleanup some vstrir define_expand naming inconsistencies
@ 2022-07-13 18:18 will schmidt
  2022-07-13 19:39 ` Segher Boessenkool
  2022-07-19 20:14 ` [PATCH, rs6000, v2] " will schmidt
  0 siblings, 2 replies; 6+ messages in thread
From: will schmidt @ 2022-07-13 18:18 UTC (permalink / raw)
  To: GCC Patches; +Cc: Segher Boessenkool, Kewen.Lin, David Edelsohn

[PATCH, rs6000] Cleanup some vstrir define_expand naming inconsistencies

Hi,
  This cleans up some of the naming around the vstrir and vstril
instruction definitions, with some cosmetic changes for consistency.
No functional changes.
Regtested just in case, no regressions.  :-)
OK for trunk?

Thanks,

gcc/
	* config/rs6000/altivec.md (vstrir_code_<mode>): Rename
	to vstrir_internal_<mode>.
	(vstrir_p_code_<mode>): Rename to vstrir_p_internal_<mode>.
	(vstril_code_<mode>): Rename to vstril_internal_<mode>.
	(vstril_p_code_<mode>): Rename to vstril_p_internal_<mode>.

diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md
index efc8ae35c2e7..5aea02e9ad6e 100644
--- a/gcc/config/rs6000/altivec.md
+++ b/gcc/config/rs6000/altivec.md
@@ -884,44 +884,44 @@ (define_expand "vstrir_<mode>"
 	(unspec:VIshort [(match_operand:VIshort 1 "altivec_register_operand")]
 			UNSPEC_VSTRIR))]
   "TARGET_POWER10"
 {
   if (BYTES_BIG_ENDIAN)
-    emit_insn (gen_vstrir_code_<mode> (operands[0], operands[1]));
+    emit_insn (gen_vstrir_internal_<mode> (operands[0], operands[1]));
   else
-    emit_insn (gen_vstril_code_<mode> (operands[0], operands[1]));
+    emit_insn (gen_vstril_internal_<mode> (operands[0], operands[1]));
   DONE;
 })
 
-(define_insn "vstrir_code_<mode>"
+(define_insn "vstrir_internal_<mode>"
   [(set (match_operand:VIshort 0 "altivec_register_operand" "=v")
 	(unspec:VIshort
 	   [(match_operand:VIshort 1 "altivec_register_operand" "v")]
 	   UNSPEC_VSTRIR))]
   "TARGET_POWER10"
   "vstri<wd>r %0,%1"
   [(set_attr "type" "vecsimple")])
 
-;; This expands into same code as vstrir_<mode> followed by condition logic
+;; This expands into same code as vstrir<mode> followed by condition logic
 ;; so that a single vstribr. or vstrihr. or vstribl. or vstrihl. instruction
 ;; can, for example, satisfy the needs of a vec_strir () function paired
 ;; with a vec_strir_p () function if both take the same incoming arguments.
 (define_expand "vstrir_p_<mode>"
   [(match_operand:SI 0 "gpc_reg_operand")
    (match_operand:VIshort 1 "altivec_register_operand")]
   "TARGET_POWER10"
 {
   rtx scratch = gen_reg_rtx (<MODE>mode);
   if (BYTES_BIG_ENDIAN)
-    emit_insn (gen_vstrir_p_code_<mode> (scratch, operands[1]));
+    emit_insn (gen_vstrir_p_internal_<mode> (scratch, operands[1]));
   else
-    emit_insn (gen_vstril_p_code_<mode> (scratch, operands[1]));
+    emit_insn (gen_vstril_p_internal_<mode> (scratch, operands[1]));
   emit_insn (gen_cr6_test_for_zero (operands[0]));
   DONE;
 })
 
-(define_insn "vstrir_p_code_<mode>"
+(define_insn "vstrir_p_internal_<mode>"
   [(set (match_operand:VIshort 0 "altivec_register_operand" "=v")
 	(unspec:VIshort
 	   [(match_operand:VIshort 1 "altivec_register_operand" "v")]
 	   UNSPEC_VSTRIR))
    (set (reg:CC CR6_REGNO)
@@ -936,17 +936,17 @@ (define_expand "vstril_<mode>"
 	(unspec:VIshort [(match_operand:VIshort 1 "altivec_register_operand")]
 			UNSPEC_VSTRIR))]
   "TARGET_POWER10"
 {
   if (BYTES_BIG_ENDIAN)
-    emit_insn (gen_vstril_code_<mode> (operands[0], operands[1]));
+    emit_insn (gen_vstril_internal_<mode> (operands[0], operands[1]));
   else
-    emit_insn (gen_vstrir_code_<mode> (operands[0], operands[1]));
+    emit_insn (gen_vstrir_internal_<mode> (operands[0], operands[1]));
   DONE;
 })
 
-(define_insn "vstril_code_<mode>"
+(define_insn "vstril_internal_<mode>"
   [(set (match_operand:VIshort 0 "altivec_register_operand" "=v")
 	(unspec:VIshort
 	   [(match_operand:VIshort 1 "altivec_register_operand" "v")]
 	   UNSPEC_VSTRIL))]
   "TARGET_POWER10"
@@ -962,18 +962,18 @@ (define_expand "vstril_p_<mode>"
    (match_operand:VIshort 1 "altivec_register_operand")]
   "TARGET_POWER10"
 {
   rtx scratch = gen_reg_rtx (<MODE>mode);
   if (BYTES_BIG_ENDIAN)
-    emit_insn (gen_vstril_p_code_<mode> (scratch, operands[1]));
+    emit_insn (gen_vstril_p_internal_<mode> (scratch, operands[1]));
   else
-    emit_insn (gen_vstrir_p_code_<mode> (scratch, operands[1]));
+    emit_insn (gen_vstrir_p_internal_<mode> (scratch, operands[1]));
   emit_insn (gen_cr6_test_for_zero (operands[0]));
   DONE;
 })
 
-(define_insn "vstril_p_code_<mode>"
+(define_insn "vstril_p_internal_<mode>"
   [(set (match_operand:VIshort 0 "altivec_register_operand" "=v")
 	(unspec:VIshort
 	   [(match_operand:VIshort 1 "altivec_register_operand" "v")]
 	   UNSPEC_VSTRIL))
    (set (reg:CC CR6_REGNO)


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH, rs6000] Cleanup some vstrir define_expand naming inconsistencies
  2022-07-13 18:18 [PATCH, rs6000] Cleanup some vstrir define_expand naming inconsistencies will schmidt
@ 2022-07-13 19:39 ` Segher Boessenkool
  2022-07-13 21:14   ` will schmidt
  2022-07-19 20:14 ` [PATCH, rs6000, v2] " will schmidt
  1 sibling, 1 reply; 6+ messages in thread
From: Segher Boessenkool @ 2022-07-13 19:39 UTC (permalink / raw)
  To: will schmidt; +Cc: GCC Patches, Kewen.Lin, David Edelsohn

Hi!

On Wed, Jul 13, 2022 at 01:18:29PM -0500, will schmidt wrote:
>   This cleans up some of the naming around the vstrir and vstril
> instruction definitions, with some cosmetic changes for consistency.

> gcc/
> 	* config/rs6000/altivec.md (vstrir_code_<mode>): Rename
> 	to vstrir_internal_<mode>.
> 	(vstrir_p_code_<mode>): Rename to vstrir_p_internal_<mode>.
> 	(vstril_code_<mode>): Rename to vstril_internal_<mode>.
> 	(vstril_p_code_<mode>): Rename to vstril_p_internal_<mode>.

It doesn't show the new names on the lhs this way.  One way to do
better
is to write e.g.
	(vstril_code_<mode>): Rename to...
	(vstril_internal_<mode>): ... this.

It often is a good idea to say "...<mode> for VIshort" and similar btw.

I'm not a fan of "internal" either, it doesn't say anything.  At least
put it at the very end of the names please?

Okay for trunk with that changed.  Thanks!


Segher

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH, rs6000] Cleanup some vstrir define_expand naming inconsistencies
  2022-07-13 19:39 ` Segher Boessenkool
@ 2022-07-13 21:14   ` will schmidt
  2022-07-13 21:26     ` Segher Boessenkool
  0 siblings, 1 reply; 6+ messages in thread
From: will schmidt @ 2022-07-13 21:14 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: GCC Patches, Kewen.Lin, David Edelsohn

On Wed, 2022-07-13 at 14:39 -0500, Segher Boessenkool wrote:
> Hi!
> 
> On Wed, Jul 13, 2022 at 01:18:29PM -0500, will schmidt wrote:
> >   This cleans up some of the naming around the vstrir and vstril
> > instruction definitions, with some cosmetic changes for
> > consistency.
> > gcc/
> > 	* config/rs6000/altivec.md (vstrir_code_<mode>): Rename
> > 	to vstrir_internal_<mode>.
> > 	(vstrir_p_code_<mode>): Rename to vstrir_p_internal_<mode>.
> > 	(vstril_code_<mode>): Rename to vstril_internal_<mode>.
> > 	(vstril_p_code_<mode>): Rename to vstril_p_internal_<mode>.
> 
> It doesn't show the new names on the lhs this way.  One way to do
> better
> is to write e.g.
> 	(vstril_code_<mode>): Rename to...
> 	(vstril_internal_<mode>): ... this.

Ok.

> 
> It often is a good idea to say "...<mode> for VIshort" and similar
> btw.

Ok. 

> 
> I'm not a fan of "internal" either, it doesn't say anything.  At
> least
> put it at the very end of the names please?
I'm easily convinced. ;-)  I wonder if I should just drop "_internal"
entirely and go with "vstrir_<mode>".  Otherwise I'll rework to be
"vstrir_<mode>_internal".
At a glance I see we do have some other existing define_insn entries
with _internal at the tail and a few others embedded in the middle. 
I'll leave a note and perhaps review those after.  :-)

Thanks,
-Will

> 
> Okay for trunk with that changed.  Thanks!
> 
> 
> Segher


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH, rs6000] Cleanup some vstrir define_expand naming inconsistencies
  2022-07-13 21:14   ` will schmidt
@ 2022-07-13 21:26     ` Segher Boessenkool
  0 siblings, 0 replies; 6+ messages in thread
From: Segher Boessenkool @ 2022-07-13 21:26 UTC (permalink / raw)
  To: will schmidt; +Cc: GCC Patches, Kewen.Lin, David Edelsohn

On Wed, Jul 13, 2022 at 04:14:11PM -0500, will schmidt wrote:
> On Wed, 2022-07-13 at 14:39 -0500, Segher Boessenkool wrote:
> > I'm not a fan of "internal" either, it doesn't say anything.  At
> > least
> > put it at the very end of the names please?
> I'm easily convinced. ;-)  I wonder if I should just drop "_internal"
> entirely and go with "vstrir_<mode>".  Otherwise I'll rework to be
> "vstrir_<mode>_internal".

The define_expand already uses that name.  Some other patterns in
altivec.md use *_direct, maybe that is nicer?

> At a glance I see we do have some other existing define_insn entries
> with _internal at the tail and a few others embedded in the middle. 
> I'll leave a note and perhaps review those after.  :-)

Thanks :-)


Segher

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH, rs6000, v2] Cleanup some vstrir define_expand naming inconsistencies
  2022-07-13 18:18 [PATCH, rs6000] Cleanup some vstrir define_expand naming inconsistencies will schmidt
  2022-07-13 19:39 ` Segher Boessenkool
@ 2022-07-19 20:14 ` will schmidt
  2022-07-20 22:15   ` Segher Boessenkool
  1 sibling, 1 reply; 6+ messages in thread
From: will schmidt @ 2022-07-19 20:14 UTC (permalink / raw)
  To: GCC Patches; +Cc: David Edelsohn, Segher Boessenkool, Kewen.Lin

[PATCH, rs6000, v2] Cleanup some vstrir define_expand naming inconsistencies

Hi,
  This cleans up some of the naming around the vstrir and vstril
instruction definitions, with some cosmetic changes for consistency.
No functional changes.
Regtested just in case, no regressions.

[V2]
Used 'direct' instead of 'internal', and cosmetically reworked
the changelog.

OK for trunk?

Thanks,

gcc/
	* config/rs6000/altivec.md:
	(vstrir_code_<mode>): Rename to...
	(vstrir_direct_<mode>): ... this.
	(vstrir_p_code_<mode>): Rename to...
	(vstrir_p_direct_<mode>): ... this.
	(vstril_code_<mode>): Rename to...
	(vstril_direct_<mode>): ... this.
	(vstril_p_code_<mode>): Rename to...
	(vstril_p_direct_<mode>): ... this.

diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md
index efc8ae35c2e7..2c4940f2e21c 100644
--- a/gcc/config/rs6000/altivec.md
+++ b/gcc/config/rs6000/altivec.md
@@ -884,44 +884,44 @@ (define_expand "vstrir_<mode>"
 	(unspec:VIshort [(match_operand:VIshort 1 "altivec_register_operand")]
 			UNSPEC_VSTRIR))]
   "TARGET_POWER10"
 {
   if (BYTES_BIG_ENDIAN)
-    emit_insn (gen_vstrir_code_<mode> (operands[0], operands[1]));
+    emit_insn (gen_vstrir_direct_<mode> (operands[0], operands[1]));
   else
-    emit_insn (gen_vstril_code_<mode> (operands[0], operands[1]));
+    emit_insn (gen_vstril_direct_<mode> (operands[0], operands[1]));
   DONE;
 })
 
-(define_insn "vstrir_code_<mode>"
+(define_insn "vstrir_direct_<mode>"
   [(set (match_operand:VIshort 0 "altivec_register_operand" "=v")
 	(unspec:VIshort
 	   [(match_operand:VIshort 1 "altivec_register_operand" "v")]
 	   UNSPEC_VSTRIR))]
   "TARGET_POWER10"
   "vstri<wd>r %0,%1"
   [(set_attr "type" "vecsimple")])
 
-;; This expands into same code as vstrir_<mode> followed by condition logic
+;; This expands into same code as vstrir<mode> followed by condition logic
 ;; so that a single vstribr. or vstrihr. or vstribl. or vstrihl. instruction
 ;; can, for example, satisfy the needs of a vec_strir () function paired
 ;; with a vec_strir_p () function if both take the same incoming arguments.
 (define_expand "vstrir_p_<mode>"
   [(match_operand:SI 0 "gpc_reg_operand")
    (match_operand:VIshort 1 "altivec_register_operand")]
   "TARGET_POWER10"
 {
   rtx scratch = gen_reg_rtx (<MODE>mode);
   if (BYTES_BIG_ENDIAN)
-    emit_insn (gen_vstrir_p_code_<mode> (scratch, operands[1]));
+    emit_insn (gen_vstrir_p_direct_<mode> (scratch, operands[1]));
   else
-    emit_insn (gen_vstril_p_code_<mode> (scratch, operands[1]));
+    emit_insn (gen_vstril_p_direct_<mode> (scratch, operands[1]));
   emit_insn (gen_cr6_test_for_zero (operands[0]));
   DONE;
 })
 
-(define_insn "vstrir_p_code_<mode>"
+(define_insn "vstrir_p_direct_<mode>"
   [(set (match_operand:VIshort 0 "altivec_register_operand" "=v")
 	(unspec:VIshort
 	   [(match_operand:VIshort 1 "altivec_register_operand" "v")]
 	   UNSPEC_VSTRIR))
    (set (reg:CC CR6_REGNO)
@@ -936,17 +936,17 @@ (define_expand "vstril_<mode>"
 	(unspec:VIshort [(match_operand:VIshort 1 "altivec_register_operand")]
 			UNSPEC_VSTRIR))]
   "TARGET_POWER10"
 {
   if (BYTES_BIG_ENDIAN)
-    emit_insn (gen_vstril_code_<mode> (operands[0], operands[1]));
+    emit_insn (gen_vstril_direct_<mode> (operands[0], operands[1]));
   else
-    emit_insn (gen_vstrir_code_<mode> (operands[0], operands[1]));
+    emit_insn (gen_vstrir_direct_<mode> (operands[0], operands[1]));
   DONE;
 })
 
-(define_insn "vstril_code_<mode>"
+(define_insn "vstril_direct_<mode>"
   [(set (match_operand:VIshort 0 "altivec_register_operand" "=v")
 	(unspec:VIshort
 	   [(match_operand:VIshort 1 "altivec_register_operand" "v")]
 	   UNSPEC_VSTRIL))]
   "TARGET_POWER10"
@@ -962,18 +962,18 @@ (define_expand "vstril_p_<mode>"
    (match_operand:VIshort 1 "altivec_register_operand")]
   "TARGET_POWER10"
 {
   rtx scratch = gen_reg_rtx (<MODE>mode);
   if (BYTES_BIG_ENDIAN)
-    emit_insn (gen_vstril_p_code_<mode> (scratch, operands[1]));
+    emit_insn (gen_vstril_p_direct_<mode> (scratch, operands[1]));
   else
-    emit_insn (gen_vstrir_p_code_<mode> (scratch, operands[1]));
+    emit_insn (gen_vstrir_p_direct_<mode> (scratch, operands[1]));
   emit_insn (gen_cr6_test_for_zero (operands[0]));
   DONE;
 })
 
-(define_insn "vstril_p_code_<mode>"
+(define_insn "vstril_p_direct_<mode>"
   [(set (match_operand:VIshort 0 "altivec_register_operand" "=v")
 	(unspec:VIshort
 	   [(match_operand:VIshort 1 "altivec_register_operand" "v")]
 	   UNSPEC_VSTRIL))
    (set (reg:CC CR6_REGNO)


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH, rs6000, v2] Cleanup some vstrir define_expand naming inconsistencies
  2022-07-19 20:14 ` [PATCH, rs6000, v2] " will schmidt
@ 2022-07-20 22:15   ` Segher Boessenkool
  0 siblings, 0 replies; 6+ messages in thread
From: Segher Boessenkool @ 2022-07-20 22:15 UTC (permalink / raw)
  To: will schmidt; +Cc: GCC Patches, David Edelsohn, Kewen.Lin

On Tue, Jul 19, 2022 at 03:14:52PM -0500, will schmidt wrote:
>   This cleans up some of the naming around the vstrir and vstril
> instruction definitions, with some cosmetic changes for consistency.

Okay for trunk.  Thanks!


Segher

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-07-20 22:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-13 18:18 [PATCH, rs6000] Cleanup some vstrir define_expand naming inconsistencies will schmidt
2022-07-13 19:39 ` Segher Boessenkool
2022-07-13 21:14   ` will schmidt
2022-07-13 21:26     ` Segher Boessenkool
2022-07-19 20:14 ` [PATCH, rs6000, v2] " will schmidt
2022-07-20 22:15   ` Segher Boessenkool

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).