diff mbox series

[rs6000] Cleanup some vstrir define_expand naming inconsistencies

Message ID 6da1e35def9d282bcf87483e78cf578fff604723.camel@vnet.ibm.com
State New
Headers
Series [rs6000] Cleanup some vstrir define_expand naming inconsistencies |

Commit Message

will schmidt July 13, 2022, 6:18 p.m. UTC 
  [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>.

Comments

Segher Boessenkool July 13, 2022, 7:39 p.m. UTC | #1 
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
will schmidt July 13, 2022, 9:14 p.m. UTC | #2 
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
Segher Boessenkool July 13, 2022, 9:26 p.m. UTC | #3 
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
diff mbox series

Patch

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)