diff mbox series

MIPS/GAS: Omit LI 0 for condition trap

Message ID 20240619170038.10146-1-syq@debian.org
State New
Headers
Series MIPS/GAS: Omit LI 0 for condition trap |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

YunQiang Su June 19, 2024, 5 p.m. UTC 
  From: YunQiang Su <syq@gcc.gnu.org>

MIPSr6 removes condition trap instructions with imm, so we expand
the instruction like "tne $2,IMM" to
	li	$at,IMM
	tne	$2,$at
While if IMM is 0, we can use
	tne	$2,$zero
only.
---
 gas/config/tc-mips.c                        | 11 ++++++++---
 gas/testsuite/gas/mips/cond-trap-imm-zero.d | 16 ++++++++++++++++
 gas/testsuite/gas/mips/cond-trap-imm-zero.s |  9 +++++++++
 gas/testsuite/gas/mips/mips.exp             |  2 ++
 4 files changed, 35 insertions(+), 3 deletions(-)
 create mode 100644 gas/testsuite/gas/mips/cond-trap-imm-zero.d
 create mode 100644 gas/testsuite/gas/mips/cond-trap-imm-zero.s

Comments

Maciej W. Rozycki June 19, 2024, 6:22 p.m. UTC | #1 
On Thu, 20 Jun 2024, YunQiang Su wrote:

> MIPSr6 removes condition trap instructions with imm, so we expand
> the instruction like "tne $2,IMM" to
> 	li	$at,IMM
> 	tne	$2,$at
> While if IMM is 0, we can use
> 	tne	$2,$zero
> only.

 Thanks for the submission.  It is indeed a missed optimisation since the 
use of the macros for the trap instructions has changed with the removal 
of the immediate forms as from the MIPSr6 ISA.  I'll have a look into it 
as there seems more to cover here.

  Maciej
YunQiang Su June 20, 2024, 3:14 a.m. UTC | #2 
Maciej W. Rozycki <macro@orcam.me.uk> 于2024年6月20日周四 02:22写道：
>
> On Thu, 20 Jun 2024, YunQiang Su wrote:
>
> > MIPSr6 removes condition trap instructions with imm, so we expand
> > the instruction like "tne $2,IMM" to
> >       li      $at,IMM
> >       tne     $2,$at
> > While if IMM is 0, we can use
> >       tne     $2,$zero
> > only.
>
>  Thanks for the submission.  It is indeed a missed optimisation since the
> use of the macros for the trap instructions has changed with the removal
> of the immediate forms as from the MIPSr6 ISA.  I'll have a look into it
> as there seems more to cover here.
>

Does you mean that other similar instructions or we still have some
more optimization for condition trap instructions?

>   Maciej
YunQiang Su June 26, 2024, 2:01 p.m. UTC | #3 
OK for trunk?

YunQiang Su <syq@debian.org> 于2024年6月20日周四 11:14写道：
>
> Maciej W. Rozycki <macro@orcam.me.uk> 于2024年6月20日周四 02:22写道：
> >
> > On Thu, 20 Jun 2024, YunQiang Su wrote:
> >
> > > MIPSr6 removes condition trap instructions with imm, so we expand
> > > the instruction like "tne $2,IMM" to
> > >       li      $at,IMM
> > >       tne     $2,$at
> > > While if IMM is 0, we can use
> > >       tne     $2,$zero
> > > only.
> >
> >  Thanks for the submission.  It is indeed a missed optimisation since the
> > use of the macros for the trap instructions has changed with the removal
> > of the immediate forms as from the MIPSr6 ISA.  I'll have a look into it
> > as there seems more to cover here.
> >
>
> Does you mean that other similar instructions or we still have some
> more optimization for condition trap instructions?
>
> >   Maciej
YunQiang Su June 27, 2024, 11:32 p.m. UTC | #4 
YunQiang Su <syq@debian.org> 于2024年6月26日周三 22:01写道：
>
> OK for trunk?
>

This is the real problem anyway ;)
diff mbox series

Patch

diff --git a/gas/config/tc-mips.c b/gas/config/tc-mips.c
index 8f54cb8937a..863f56d5f8c 100644
--- a/gas/config/tc-mips.c
+++ b/gas/config/tc-mips.c
@@ -13815,9 +13815,14 @@  macro (struct mips_cl_insn *ip, char *str)
     case M_TNE_I:
       s = "tne";
     trap:
-      used_at = 1;
-      load_register (AT, &imm_expr, GPR_SIZE == 64);
-      macro_build (NULL, s, "s,t", op[0], AT);
+      if (imm_expr.X_add_number != 0)
+	{
+	  used_at = 1;
+	  load_register (AT, &imm_expr, GPR_SIZE == 64);
+	  macro_build (NULL, s, "s,t", op[0], AT);
+	}
+      else
+	  macro_build (NULL, s, "s,t", op[0], ZERO);
       break;
 
     case M_TRUNCWS:
diff --git a/gas/testsuite/gas/mips/cond-trap-imm-zero.d b/gas/testsuite/gas/mips/cond-trap-imm-zero.d
new file mode 100644
index 00000000000..39d4c9609b2
--- /dev/null
+++ b/gas/testsuite/gas/mips/cond-trap-imm-zero.d
@@ -0,0 +1,16 @@ 
+#objdump: -dr
+#name: Condition Trap convert IMM0 to REG0 (MIPSr6)
+#as: -32
+
+.*: +file format .*
+
+Disassembly of section \.text:
+
+00000000 <f>:
+   0:	00c00034 	teq	a2,zero
+   4:	00c00030 	tge	a2,zero
+   8:	00c00031 	tgeu	a2,zero
+   c:	00c00032 	tlt	a2,zero
+  10:	00c00033 	tltu	a2,zero
+  14:	00c00036 	tne	a2,zero
+	\.\.\.
diff --git a/gas/testsuite/gas/mips/cond-trap-imm-zero.s b/gas/testsuite/gas/mips/cond-trap-imm-zero.s
new file mode 100644
index 00000000000..84cc7f1f842
--- /dev/null
+++ b/gas/testsuite/gas/mips/cond-trap-imm-zero.s
@@ -0,0 +1,9 @@ 
+        .set    noreorder
+        .set    nomacro
+f:
+        teq	$6,0
+        tge	$6,0
+        tgeu	$6,0
+	tlt	$6,0
+	tltu	$6,0
+        tne	$6,0
diff --git a/gas/testsuite/gas/mips/mips.exp b/gas/testsuite/gas/mips/mips.exp
index f657b0e17a3..6ec10ddb165 100644
--- a/gas/testsuite/gas/mips/mips.exp
+++ b/gas/testsuite/gas/mips/mips.exp
@@ -2180,4 +2180,6 @@  if { [istarget mips*-*-vxworks*] } {
     if [istarget *-*-irix*] {
       run_dump_test "irix-no-pdr"
     }
+
+    run_dump_test_arches "cond-trap-imm-zero" [mips_arch_list_matching mips32r6]
 }