Fix typo in insn name.

Message ID ZKxjMDOl1K/d5z23@cowardly-lion.the-meissners.org
State Accepted
Headers
Series Fix typo in insn name. |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Michael Meissner July 10, 2023, 7:59 p.m. UTC
  In doing other work, I noticed that there was an insn:

	vsx_extract_v4sf_<mode>_load

Which did not have an iterator.  I removed the useless <mode>.

I have tested this patch on the following systems and there was no degration.
Can I check it into the trunk branch?

    *	Power10, LE, --with-cpu=power10, IBM 128-bit long double
    *	Power9,  LE, --with-cpu=power9,  IBM 128-bit long double
    *	Power9,  LE, --with-cpu=power9,  IEEE 128-bit long double
    *   Power9,  LE, --with-cpu=power9,  64-bit default long double
    *	Power9,  BE, --with-cpu=power9,  IBM 128-bit long double
    *	Power8,  BE, --with-cpu=power8,  IBM 128-bit long double

2023-07-10  Michael Meissner  <meissner@linux.ibm.com>

gcc/

	* config/rs6000/vsx.md (vsx_extract_v4sf_load): Rename from
	vsx_extract_v4sf_<mode>_load.
---
 gcc/config/rs6000/vsx.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Segher Boessenkool July 10, 2023, 8:10 p.m. UTC | #1
Hi!

On Mon, Jul 10, 2023 at 03:59:44PM -0400, Michael Meissner wrote:
> In doing other work, I noticed that there was an insn:
> 
> 	vsx_extract_v4sf_<mode>_load
> 
> Which did not have an iterator.  I removed the useless <mode>.

This patch does that, you mean.

> --- a/gcc/config/rs6000/vsx.md
> +++ b/gcc/config/rs6000/vsx.md
> @@ -3576,7 +3576,7 @@ (define_insn_and_split "vsx_extract_v4sf"
>    [(set_attr "length" "8")
>     (set_attr "type" "fp")])
>  
> -(define_insn_and_split "*vsx_extract_v4sf_<mode>_load"
> +(define_insn_and_split "*vsx_extract_v4sf_load"
>    [(set (match_operand:SF 0 "register_operand" "=f,v,v,?r")
>  	(vec_select:SF
>  	 (match_operand:V4SF 1 "memory_operand" "m,Z,m,m")

Does this fix any ICEs?  Or do you have some example that makes better
machine code after this change?  Or would a better change perhaps be to
just remove this pattern completely, if it doesn't do anything useful?

I.e., please include a new testcase.


Segher
  
Michael Meissner July 10, 2023, 10:25 p.m. UTC | #2
On Mon, Jul 10, 2023 at 03:10:21PM -0500, Segher Boessenkool wrote:
> Hi!
> 
> On Mon, Jul 10, 2023 at 03:59:44PM -0400, Michael Meissner wrote:
> > In doing other work, I noticed that there was an insn:
> > 
> > 	vsx_extract_v4sf_<mode>_load
> > 
> > Which did not have an iterator.  I removed the useless <mode>.
> 
> This patch does that, you mean.
> 
> > --- a/gcc/config/rs6000/vsx.md
> > +++ b/gcc/config/rs6000/vsx.md
> > @@ -3576,7 +3576,7 @@ (define_insn_and_split "vsx_extract_v4sf"
> >    [(set_attr "length" "8")
> >     (set_attr "type" "fp")])
> >  
> > -(define_insn_and_split "*vsx_extract_v4sf_<mode>_load"
> > +(define_insn_and_split "*vsx_extract_v4sf_load"
> >    [(set (match_operand:SF 0 "register_operand" "=f,v,v,?r")
> >  	(vec_select:SF
> >  	 (match_operand:V4SF 1 "memory_operand" "m,Z,m,m")
> 
> Does this fix any ICEs?  Or do you have some example that makes better
> machine code after this change?  Or would a better change perhaps be to
> just remove this pattern completely, if it doesn't do anything useful?
> 
> I.e., please include a new testcase.

There is absolutely no code change.  It is purely a cleanup patch.  In doing
other patches, I just noticed that pattern had a _<mode> in it when it didn't
have an iterator.  I just cleaned up the code removing _<mode>.  I probably
should have changed it to vsx_extract_v4sf_sf_load.
  
Michael Meissner July 24, 2023, 6:44 p.m. UTC | #3
Ping clean-up patch.

| Date: Mon, 10 Jul 2023 15:59:44 -0400
| From: Michael Meissner <meissner@linux.ibm.com>
| Subject: [PATCH] Fix typo in insn name.
| Message-ID: <ZKxjMDOl1K/d5z23@cowardly-lion.the-meissners.org>

As I said in the reply, the only thing this patch does is to rename
vsx_extract_v4sf_<mode>_load to vsx_extract_v4sf_load since the insn does not
use a mode iterator.
  
Kewen.Lin July 26, 2023, 5:54 a.m. UTC | #4
Hi Mike,

on 2023/7/11 03:59, Michael Meissner wrote:
> In doing other work, I noticed that there was an insn:
> 
> 	vsx_extract_v4sf_<mode>_load
> 
> Which did not have an iterator.  I removed the useless <mode>.

It actually has a mode iterator, the "P" is used for clobber.

The whole pattern of this define_insn_and_split is

(define_insn_and_split "*vsx_extract_v4sf_<mode>_load"
  [(set (match_operand:SF 0 "register_operand" "=f,v,v,?r")
	(vec_select:SF
	 (match_operand:V4SF 1 "memory_operand" "m,Z,m,m")
	 (parallel [(match_operand:QI 2 "const_0_to_3_operand" "n,n,n,n")])))
   (clobber (match_scratch:P 3 "=&b,&b,&b,&b"))]         <== *P used here*

Its definition is:

(define_mode_iterator P [(SI "TARGET_32BIT") (DI "TARGET_64BIT")])

I guess we can just leave it there?

BR,
Kewen

> 
> I have tested this patch on the following systems and there was no degration.
> Can I check it into the trunk branch?
> 
>     *	Power10, LE, --with-cpu=power10, IBM 128-bit long double
>     *	Power9,  LE, --with-cpu=power9,  IBM 128-bit long double
>     *	Power9,  LE, --with-cpu=power9,  IEEE 128-bit long double
>     *   Power9,  LE, --with-cpu=power9,  64-bit default long double
>     *	Power9,  BE, --with-cpu=power9,  IBM 128-bit long double
>     *	Power8,  BE, --with-cpu=power8,  IBM 128-bit long double
> 
> 2023-07-10  Michael Meissner  <meissner@linux.ibm.com>
> 
> gcc/
> 
> 	* config/rs6000/vsx.md (vsx_extract_v4sf_load): Rename from
> 	vsx_extract_v4sf_<mode>_load.
> ---
>  gcc/config/rs6000/vsx.md | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
> index d34c3b21abe..aed450e31ec 100644
> --- a/gcc/config/rs6000/vsx.md
> +++ b/gcc/config/rs6000/vsx.md
> @@ -3576,7 +3576,7 @@ (define_insn_and_split "vsx_extract_v4sf"
>    [(set_attr "length" "8")
>     (set_attr "type" "fp")])
>  
> -(define_insn_and_split "*vsx_extract_v4sf_<mode>_load"
> +(define_insn_and_split "*vsx_extract_v4sf_load"
>    [(set (match_operand:SF 0 "register_operand" "=f,v,v,?r")
>  	(vec_select:SF
>  	 (match_operand:V4SF 1 "memory_operand" "m,Z,m,m")
  
Michael Meissner July 26, 2023, 9:53 p.m. UTC | #5
On Wed, Jul 26, 2023 at 01:54:01PM +0800, Kewen.Lin wrote:
> Hi Mike,
> 
> on 2023/7/11 03:59, Michael Meissner wrote:
> > In doing other work, I noticed that there was an insn:
> > 
> > 	vsx_extract_v4sf_<mode>_load
> > 
> > Which did not have an iterator.  I removed the useless <mode>.
> 
> It actually has a mode iterator, the "P" is used for clobber.
> 
> The whole pattern of this define_insn_and_split is
> 
> (define_insn_and_split "*vsx_extract_v4sf_<mode>_load"
>   [(set (match_operand:SF 0 "register_operand" "=f,v,v,?r")
> 	(vec_select:SF
> 	 (match_operand:V4SF 1 "memory_operand" "m,Z,m,m")
> 	 (parallel [(match_operand:QI 2 "const_0_to_3_operand" "n,n,n,n")])))
>    (clobber (match_scratch:P 3 "=&b,&b,&b,&b"))]         <== *P used here*
> 
> Its definition is:
> 
> (define_mode_iterator P [(SI "TARGET_32BIT") (DI "TARGET_64BIT")])
> 
> I guess we can just leave it there?
> 
> BR,
> Kewen

Yes, I didn't notice the :P in the insn.
  

Patch

diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
index d34c3b21abe..aed450e31ec 100644
--- a/gcc/config/rs6000/vsx.md
+++ b/gcc/config/rs6000/vsx.md
@@ -3576,7 +3576,7 @@  (define_insn_and_split "vsx_extract_v4sf"
   [(set_attr "length" "8")
    (set_attr "type" "fp")])
 
-(define_insn_and_split "*vsx_extract_v4sf_<mode>_load"
+(define_insn_and_split "*vsx_extract_v4sf_load"
   [(set (match_operand:SF 0 "register_operand" "=f,v,v,?r")
 	(vec_select:SF
 	 (match_operand:V4SF 1 "memory_operand" "m,Z,m,m")