MIPS: Use Reg0 instead of const0_rtx for TRAP

Message ID 20240619155323.7112-1-syq@gcc.gnu.org
State New
Headers
Series MIPS: Use Reg0 instead of const0_rtx for TRAP |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Test passed

Commit Message

YunQiang Su June 19, 2024, 3:53 p.m. UTC
  MIPSr6 removes condition trap instructions with imm, so the instruction
like `teq $2,imm` will be converted to
  li $at, imm
  teq $2, $at

The current version of Gas cannot detect if imm is zero, and output
  teq $2, $0
Let's do it in GCC.

gcc
	* config/mips/mips.cc(mips_expand_conditional_trap): Use Reg0
	instead of const0_rtx.
---
 gcc/config/mips/mips.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Maciej W. Rozycki June 19, 2024, 5:23 p.m. UTC | #1
On Wed, 19 Jun 2024, YunQiang Su wrote:

> MIPSr6 removes condition trap instructions with imm, so the instruction
> like `teq $2,imm` will be converted to
>   li $at, imm
>   teq $2, $at
> 
> The current version of Gas cannot detect if imm is zero, and output
>   teq $2, $0
> Let's do it in GCC.

 It seems like an output pattern issue with `*conditional_trap_reg<mode>' 
insn to me.

> diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
> index 48924116937..ba1e6214656 100644
> --- a/gcc/config/mips/mips.cc
> +++ b/gcc/config/mips/mips.cc
> @@ -6026,7 +6026,7 @@ mips_expand_conditional_trap (rtx comparison)
>  
>    emit_insn (gen_rtx_TRAP_IF (VOIDmode,
>  			      gen_rtx_fmt_ee (code, mode, op0, op1),
> -			      const0_rtx));
> +			      gen_rtx_REG (mode, GP_REG_FIRST)));

 IOW this just papers over the actual issue.

 FWIW,

  Maciej
  
YunQiang Su June 20, 2024, 3:20 a.m. UTC | #2
Maciej W. Rozycki <macro@orcam.me.uk> 于2024年6月20日周四 01:24写道:
>
> On Wed, 19 Jun 2024, YunQiang Su wrote:
>
> > MIPSr6 removes condition trap instructions with imm, so the instruction
> > like `teq $2,imm` will be converted to
> >   li $at, imm
> >   teq $2, $at
> >
> > The current version of Gas cannot detect if imm is zero, and output
> >   teq $2, $0
> > Let's do it in GCC.
>
>  It seems like an output pattern issue with `*conditional_trap_reg<mode>'
> insn to me.
>

Yes. You are right. We should update `*conditional_trap_reg<mode>'.

> > diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
> > index 48924116937..ba1e6214656 100644
> > --- a/gcc/config/mips/mips.cc
> > +++ b/gcc/config/mips/mips.cc
> > @@ -6026,7 +6026,7 @@ mips_expand_conditional_trap (rtx comparison)
> >
> >    emit_insn (gen_rtx_TRAP_IF (VOIDmode,
> >                             gen_rtx_fmt_ee (code, mode, op0, op1),
> > -                           const0_rtx));
> > +                           gen_rtx_REG (mode, GP_REG_FIRST)));
>
>  IOW this just papers over the actual issue.
>

I think that we still need it, as it will make the RTL more easy to understand.
I think that we should make the surprise in RTL as less as possible.

>  FWIW,
>
>   Maciej
  
YunQiang Su June 20, 2024, 5:11 a.m. UTC | #3
YunQiang Su <syq@gcc.gnu.org> 于2024年6月20日周四 11:20写道:
>
> Maciej W. Rozycki <macro@orcam.me.uk> 于2024年6月20日周四 01:24写道:
> >
> > On Wed, 19 Jun 2024, YunQiang Su wrote:
> >
> > > MIPSr6 removes condition trap instructions with imm, so the instruction
> > > like `teq $2,imm` will be converted to
> > >   li $at, imm
> > >   teq $2, $at
> > >
> > > The current version of Gas cannot detect if imm is zero, and output
> > >   teq $2, $0
> > > Let's do it in GCC.
> >
> >  It seems like an output pattern issue with `*conditional_trap_reg<mode>'
> > insn to me.
> >
>
> Yes. You are right. We should update `*conditional_trap_reg<mode>'.
>
> > > diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
> > > index 48924116937..ba1e6214656 100644
> > > --- a/gcc/config/mips/mips.cc
> > > +++ b/gcc/config/mips/mips.cc
> > > @@ -6026,7 +6026,7 @@ mips_expand_conditional_trap (rtx comparison)
> > >
> > >    emit_insn (gen_rtx_TRAP_IF (VOIDmode,
> > >                             gen_rtx_fmt_ee (code, mode, op0, op1),
> > > -                           const0_rtx));
> > > +                           gen_rtx_REG (mode, GP_REG_FIRST)));
> >
> >  IOW this just papers over the actual issue.
> >
>
> I think that we still need it, as it will make the RTL more easy to understand.
> I think that we should make the surprise in RTL as less as possible.
>

Ohh, you are right. It seems some RTL optimization passes prefers const0_rtx
much more. It is not easy to use REG0 here.

> >  FWIW,
> >
> >   Maciej
  

Patch

diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
index 48924116937..ba1e6214656 100644
--- a/gcc/config/mips/mips.cc
+++ b/gcc/config/mips/mips.cc
@@ -6026,7 +6026,7 @@  mips_expand_conditional_trap (rtx comparison)
 
   emit_insn (gen_rtx_TRAP_IF (VOIDmode,
 			      gen_rtx_fmt_ee (code, mode, op0, op1),
-			      const0_rtx));
+			      gen_rtx_REG (mode, GP_REG_FIRST)));
 }
 
 /* Initialize *CUM for a call to a function of type FNTYPE.  */