[PATCH] regrename: Skip renaming if instruction is noop move.

[PATCH] regrename: Skip renaming if instruction is noop move.

[Jojo R]

Skip renaming if instruction is noop move, and it will
been removed for performance.

gcc/
	* regrename.c (find_rename_reg): Return satisfied regno
	if instruction is noop move.
---
 gcc/regrename.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gcc/regrename.c b/gcc/regrename.c
index b8a9ca36f22..cb605f5176b 100644
--- a/gcc/regrename.c
+++ b/gcc/regrename.c
@@ -394,6 +394,9 @@ find_rename_reg (du_head_p this_head, enum reg_class super_class,
 			  this_head, *unavailable))
     return this_head->tied_chain->regno;
 
+  if (noop_move_p (this_head->first->insn))
+    return best_new_reg;
+
   /* If PREFERRED_CLASS is not NO_REGS, we iterate in the first pass
      over registers that belong to PREFERRED_CLASS and try to find the
      best register within the class.  If that failed, we iterate in
-- 
2.24.3 (Apple Git-128)

Re: [PATCH] regrename: Skip renaming if instruction is noop move.

[Richard Biener]

On Tue, Nov 16, 2021 at 12:45 PM Jojo R via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Skip renaming if instruction is noop move, and it will
> been removed for performance.

Is there any (target specific) testcase you can add?  Such commits are
problematic
when later bisected to since the intent isn't clear.

> gcc/
>         * regrename.c (find_rename_reg): Return satisfied regno
>         if instruction is noop move.
> ---
>  gcc/regrename.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/gcc/regrename.c b/gcc/regrename.c
> index b8a9ca36f22..cb605f5176b 100644
> --- a/gcc/regrename.c
> +++ b/gcc/regrename.c
> @@ -394,6 +394,9 @@ find_rename_reg (du_head_p this_head, enum reg_class super_class,
>                           this_head, *unavailable))
>      return this_head->tied_chain->regno;
>
> +  if (noop_move_p (this_head->first->insn))
> +    return best_new_reg;
> +
>    /* If PREFERRED_CLASS is not NO_REGS, we iterate in the first pass
>       over registers that belong to PREFERRED_CLASS and try to find the
>       best register within the class.  If that failed, we iterate in
> --
> 2.24.3 (Apple Git-128)
>

Re: [PATCH] regrename: Skip renaming if instruction is noop move.

[Jojo R]

— Jojo
在 2021年11月16日 +0800 PM8:12，Richard Biener <richard.guenther@gmail.com>，写道：
> On Tue, Nov 16, 2021 at 12:45 PM Jojo R via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > Skip renaming if instruction is noop move, and it will
> > been removed for performance.
>
> Is there any (target specific) testcase you can add? Such commits are
> problematic
> when later bisected to since the intent isn't clear.

I made a issue in bugzilla, please check it, thanks.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103296
>
> > gcc/
> > * regrename.c (find_rename_reg): Return satisfied regno
> > if instruction is noop move.
> > ---
> > gcc/regrename.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/gcc/regrename.c b/gcc/regrename.c
> > index b8a9ca36f22..cb605f5176b 100644
> > --- a/gcc/regrename.c
> > +++ b/gcc/regrename.c
> > @@ -394,6 +394,9 @@ find_rename_reg (du_head_p this_head, enum reg_class super_class,
> > this_head, *unavailable))
> > return this_head->tied_chain->regno;
> >
> > + if (noop_move_p (this_head->first->insn))
> > + return best_new_reg;
> > +
> > /* If PREFERRED_CLASS is not NO_REGS, we iterate in the first pass
> > over registers that belong to PREFERRED_CLASS and try to find the
> > best register within the class. If that failed, we iterate in
> > --
> > 2.24.3 (Apple Git-128)

Re: [PATCH] regrename: Skip renaming if instruction is noop move.

[Jeff Law]

On 11/16/2021 7:20 PM, Jojo R via Gcc-patches wrote:
> — Jojo
> 在 2021年11月16日 +0800 PM8:12，Richard Biener <richard.guenther@gmail.com>，写道：
>> On Tue, Nov 16, 2021 at 12:45 PM Jojo R via Gcc-patches
>> <gcc-patches@gcc.gnu.org> wrote:
>>> Skip renaming if instruction is noop move, and it will
>>> been removed for performance.
>> Is there any (target specific) testcase you can add? Such commits are
>> problematic
>> when later bisected to since the intent isn't clear.
> I made a issue in bugzilla, please check it, thanks.
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103296
So what Richi is asking is can you construct a testcase for the 
testsuite?  Having a BZ is helpful because we can reference it in the 
commit message, but a test, even if it's target specific, is even better 
from a long term maintenance standpoint.

Jeff

Re: [PATCH] regrename: Skip renaming if instruction is noop move.

[Jojo R]

— Jojo
在 2021年11月19日 +0800 AM12:13，Jeff Law <jeffreyalaw@gmail.com>，写道：
>
>
> On 11/16/2021 7:20 PM, Jojo R via Gcc-patches wrote:
> > — Jojo
> > 在 2021年11月16日 +0800 PM8:12，Richard Biener <richard.guenther@gmail.com>，写道：
> > > On Tue, Nov 16, 2021 at 12:45 PM Jojo R via Gcc-patches
> > > <gcc-patches@gcc.gnu.org> wrote:
> > > > Skip renaming if instruction is noop move, and it will
> > > > been removed for performance.
> > > Is there any (target specific) testcase you can add? Such commits are
> > > problematic
> > > when later bisected to since the intent isn't clear.
> > I made a issue in bugzilla, please check it, thanks.
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103296
> So what Richi is asking is can you construct a testcase for the
> testsuite?  Having a BZ is helpful because we can reference it in the
> commit message, but a test, even if it's target specific, is even better
> from a long term maintenance standpoint.
>
I found this issue from the ISA extension vector of risc-v target, and
It has not been upstream by now, normal test case without vector isa
Is difficult to construct for this patch, but I think it’s simple and useful for
other ISAs or targets, or recommit this patch after our risc-v Vector ISA
Is ready on master branch ?

Any suggestions ?
> Jeff

Re: [PATCH] regrename: Skip renaming if instruction is noop move.

[Jeff Law]

On 11/18/2021 11:23 PM, Jojo R wrote:
>
> — Jojo
> 在 2021年11月19日 +0800 AM12:13，Jeff Law <jeffreyalaw@gmail.com>，写道：
>
>
>
>     On 11/16/2021 7:20 PM, Jojo R via Gcc-patches wrote:
>
>         — Jojo
>         在 2021年11月16日 +0800 PM8:12，Richard Biener
>         <richard.guenther@gmail.com>，写道：
>
>             On Tue, Nov 16, 2021 at 12:45 PM Jojo R via Gcc-patches
>             <gcc-patches@gcc.gnu.org> wrote:
>
>                 Skip renaming if instruction is noop move, and it will
>                 been removed for performance.
>
>             Is there any (target specific) testcase you can add? Such
>             commits are
>             problematic
>             when later bisected to since the intent isn't clear.
>
>         I made a issue in bugzilla, please check it, thanks.
>
>         https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103296
>
>     So what Richi is asking is can you construct a testcase for the
>     testsuite?  Having a BZ is helpful because we can reference it in the
>     commit message, but a test, even if it's target specific, is even
>     better
>     from a long term maintenance standpoint.
>
> I found this issue from the ISA extension vector of risc-v target, and
> It has not been upstream by now, normal test case without vector isa
> Is difficult to construct for this patch, but I think it’s simple and 
> useful for
> other ISAs or targets, or recommit this patch after our risc-v Vector ISA
> Is ready on master branch ?
>
> Any suggestions ?
So I tried to trigger this on x86, but wasn't able with relatively light 
testing.

My recommendation would be to add a little comment like this before the 
change:

/* If this insn is a noop move, then do not rename in this chain as doing so
     would inhibit removal of the noop move.  */

OK with that change.

Thanks for your patience.

jeff

[PATCH v2] regrename: Skip renaming if instruction is noop move.

[Jojo R]

Skip renaming if instruction is noop move, and it will
been removed for performance.

gcc/
	* regrename.c (find_rename_reg): Return satisfied regno
	if instruction is noop move.
---
 gcc/regrename.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gcc/regrename.c b/gcc/regrename.c
index b8a9ca36f22..fe72fcc3624 100644
--- a/gcc/regrename.c
+++ b/gcc/regrename.c
@@ -394,6 +394,11 @@ find_rename_reg (du_head_p this_head, enum reg_class super_class,
 			  this_head, *unavailable))
     return this_head->tied_chain->regno;
 
+  /* If this insn is a noop move, then do not rename in this chain as doing so
+     would inhibit removal of the noop move.  */
+  if (noop_move_p (this_head->first->insn))
+    return best_new_reg;
+
   /* If PREFERRED_CLASS is not NO_REGS, we iterate in the first pass
      over registers that belong to PREFERRED_CLASS and try to find the
      best register within the class.  If that failed, we iterate in
-- 
2.24.3 (Apple Git-128)

Re: [PATCH v2] regrename: Skip renaming if instruction is noop move.

[Jeff Law]

On 12/2/2021 9:26 PM, Jojo R wrote:
> Skip renaming if instruction is noop move, and it will
> been removed for performance.
>
> gcc/
> 	* regrename.c (find_rename_reg): Return satisfied regno
> 	if instruction is noop move.
OK
jeff

Re: [PATCH v2] regrename: Skip renaming if instruction is noop move.

[Jojo R]

Hi,

	Thank you for your review & help.

	I could not fetch the merged patch from gcc master of git.

	Is there any problem for this ?

Thanks.

— Jojo
在 2021年12月3日 +0800 PM10:57，Jeff Law <jeffreyalaw@gmail.com>，写道：
>
>
> On 12/2/2021 9:26 PM, Jojo R wrote:
> > Skip renaming if instruction is noop move, and it will
> > been removed for performance.
> >
> > gcc/
> > * regrename.c (find_rename_reg): Return satisfied regno
> > if instruction is noop move.
> OK
> jeff

Re: [PATCH v2] regrename: Skip renaming if instruction is noop move.

[Jeff Law]

On 12/13/2021 6:40 PM, Jojo R wrote:
> Hi,
>
> Thank you for your review & help.
>
> I could not fetch the merged patch from gcc master of git.
>
> Is there any problem for this ?
I assumed you'd commit the change.  I thought you had commit 
privileges.   I'll go ahead and push it momentarily.

Thanks for following up.

Jeff

[PATCH v2] regrename: Skip renaming if instruction is noop move.

[Jojo R]

Skip renaming if instruction is noop move, and it will
been removed for performance.

gcc/
	* regrename.c (find_rename_reg): Return satisfied regno
	if instruction is noop move.
---
 gcc/regrename.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gcc/regrename.c b/gcc/regrename.c
index b8a9ca36f22..fe72fcc3624 100644
--- a/gcc/regrename.c
+++ b/gcc/regrename.c
@@ -394,6 +394,11 @@ find_rename_reg (du_head_p this_head, enum reg_class super_class,
 			  this_head, *unavailable))
     return this_head->tied_chain->regno;
 
+  /* If this insn is a noop move, then do not rename in this chain as doing so
+     would inhibit removal of the noop move.  */
+  if (noop_move_p (this_head->first->insn))
+    return best_new_reg;
+
   /* If PREFERRED_CLASS is not NO_REGS, we iterate in the first pass
      over registers that belong to PREFERRED_CLASS and try to find the
      best register within the class.  If that failed, we iterate in
-- 
2.24.3 (Apple Git-128)