diff mbox series

RISC-V: Fix PR109535

Message ID 20230418020311.36368-1-juzhe.zhong@rivai.ai
State Accepted
Headers
Series RISC-V: Fix PR109535 |

Checks

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

Commit Message

juzhe.zhong@rivai.ai April 18, 2023, 2:03 a.m. UTC 
  From: Ju-Zhe Zhong <juzhe.zhong@rivai.ai>

gcc/ChangeLog:

        * config/riscv/riscv-vsetvl.cc (count_regno_occurrences): New function.
        (pass_vsetvl::cleanup_insns): Fix bug.

---
 gcc/config/riscv/riscv-vsetvl.cc | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Comments

Jeff Law April 18, 2023, 7 p.m. UTC | #1 
On 4/17/23 20:03, juzhe.zhong@rivai.ai wrote:
> From: Ju-Zhe Zhong <juzhe.zhong@rivai.ai>
> 
> gcc/ChangeLog:
> 
>          * config/riscv/riscv-vsetvl.cc (count_regno_occurrences): New function.
>          (pass_vsetvl::cleanup_insns): Fix bug.
ChangeLog should reference the bug number, like this:

	PR target/109535


> 
> ---
>   gcc/config/riscv/riscv-vsetvl.cc | 15 ++++++++++++++-
>   1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
> index 1b66e3b9eeb..43e2cf08377 100644
> --- a/gcc/config/riscv/riscv-vsetvl.cc
> +++ b/gcc/config/riscv/riscv-vsetvl.cc
> @@ -1592,6 +1592,19 @@ backward_propagate_worthwhile_p (const basic_block cfg_bb,
>     return true;
>   }
>   
> +/* Count the number of REGNO in RINSN.  */
> +int
> +count_regno_occurrences (rtx_insn *rinsn, unsigned int regno)
Seems like this ought to be static. Though it's not clear why 
count_occurrences didn't do what you needed.  Can you explain why 
count_occurrences was insufficient for your needs?




Jeff
Kito Cheng April 18, 2023, 7:03 p.m. UTC | #2 
Hi Jeff, Ju-Zhe:

Let you know that I am running creduce with this testcase for reduce
the size of testcsae, it's really too huge...

On Wed, Apr 19, 2023 at 3:00 AM Jeff Law via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
>
> On 4/17/23 20:03, juzhe.zhong@rivai.ai wrote:
> > From: Ju-Zhe Zhong <juzhe.zhong@rivai.ai>
> >
> > gcc/ChangeLog:
> >
> >          * config/riscv/riscv-vsetvl.cc (count_regno_occurrences): New function.
> >          (pass_vsetvl::cleanup_insns): Fix bug.
> ChangeLog should reference the bug number, like this:
>
>         PR target/109535
>
>
> >
> > ---
> >   gcc/config/riscv/riscv-vsetvl.cc | 15 ++++++++++++++-
> >   1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
> > index 1b66e3b9eeb..43e2cf08377 100644
> > --- a/gcc/config/riscv/riscv-vsetvl.cc
> > +++ b/gcc/config/riscv/riscv-vsetvl.cc
> > @@ -1592,6 +1592,19 @@ backward_propagate_worthwhile_p (const basic_block cfg_bb,
> >     return true;
> >   }
> >
> > +/* Count the number of REGNO in RINSN.  */
> > +int
> > +count_regno_occurrences (rtx_insn *rinsn, unsigned int regno)
> Seems like this ought to be static. Though it's not clear why
> count_occurrences didn't do what you needed.  Can you explain why
> count_occurrences was insufficient for your needs?
>
>
>
>
> Jeff
juzhe.zhong@rivai.ai April 18, 2023, 10:53 p.m. UTC | #3 
>> ChangeLog should reference the bug number, like this:
>> PR target/109535
>> Seems like this ought to be static. Though it's not clear why
>> count_occurrences didn't do what you needed.  Can you explain why
>> count_occurrences was insufficient for your needs?

Address comment, I will resend a patch with referencing bug PR number and adding "static".
The reason why count_occurrences can not work since we want to count the regno occurrences
instead of rtx occurrences.

The bug issue reported by google/highway project:
(set(..........)
       (reg:QI s0)
        (reg:DI s0))

The "avl" operand rtx  = (reg:DI s0)
count_occurrences return 1 however the actual regno occurrences should be 2.

Thanks


juzhe.zhong@rivai.ai
 
From: Jeff Law
Date: 2023-04-19 03:00
To: juzhe.zhong; gcc-patches
CC: kito.cheng; palmer
Subject: Re: [PATCH] RISC-V: Fix PR109535
 
 
On 4/17/23 20:03, juzhe.zhong@rivai.ai wrote:
> From: Ju-Zhe Zhong <juzhe.zhong@rivai.ai>
> 
> gcc/ChangeLog:
> 
>          * config/riscv/riscv-vsetvl.cc (count_regno_occurrences): New function.
>          (pass_vsetvl::cleanup_insns): Fix bug.
ChangeLog should reference the bug number, like this:
 
PR target/109535
 
 
> 
> ---
>   gcc/config/riscv/riscv-vsetvl.cc | 15 ++++++++++++++-
>   1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
> index 1b66e3b9eeb..43e2cf08377 100644
> --- a/gcc/config/riscv/riscv-vsetvl.cc
> +++ b/gcc/config/riscv/riscv-vsetvl.cc
> @@ -1592,6 +1592,19 @@ backward_propagate_worthwhile_p (const basic_block cfg_bb,
>     return true;
>   }
>   
> +/* Count the number of REGNO in RINSN.  */
> +int
> +count_regno_occurrences (rtx_insn *rinsn, unsigned int regno)
Seems like this ought to be static. Though it's not clear why 
count_occurrences didn't do what you needed.  Can you explain why 
count_occurrences was insufficient for your needs?
 
 
 
 
Jeff
diff mbox series

Patch

diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
index 1b66e3b9eeb..43e2cf08377 100644
--- a/gcc/config/riscv/riscv-vsetvl.cc
+++ b/gcc/config/riscv/riscv-vsetvl.cc
@@ -1592,6 +1592,19 @@  backward_propagate_worthwhile_p (const basic_block cfg_bb,
   return true;
 }
 
+/* Count the number of REGNO in RINSN.  */
+int
+count_regno_occurrences (rtx_insn *rinsn, unsigned int regno)
+{
+  int count = 0;
+  extract_insn (rinsn);
+  for (int i = 0; i < recog_data.n_operands; i++)
+    if (REG_P (recog_data.operand[i])
+	&& REGNO (recog_data.operand[i]) == regno)
+      count++;
+  return count;
+}
+
 avl_info::avl_info (const avl_info &other)
 {
   m_value = other.get_value ();
@@ -3924,7 +3937,7 @@  pass_vsetvl::cleanup_insns (void) const
 	  if (!has_vl_op (rinsn) || !REG_P (get_vl (rinsn)))
 	    continue;
 	  rtx avl = get_vl (rinsn);
-	  if (count_occurrences (PATTERN (rinsn), avl, 0) == 1)
+	  if (count_regno_occurrences (rinsn, REGNO (avl)) == 1)
 	    {
 	      /* Get the list of uses for the new instruction.  */
 	      auto attempt = crtl->ssa->new_change_attempt ();