Re: [PATCH, MIPS] Add baddu Octeon instruction

[Adam Nemet <anemet at caviumnetworks dot com>]

Richard Sandiford wrote:
> (define_predicate "truncate_operator"
>   (ior (and (match_code "subreg")
>             (match_test "subreg_lowpart_p (op)")
>             (match_test "TRULY_NOOP_TRUNCATION
>                           (GET_MODE_BITSIZE (GET_MODE (op)),
>                            GET_MODE_BITSIZE (GET_MODE (SUBREG_REG (op))))"))
>        (match_code "truncate")))
> 
> ?  Then a single pattern:
> 
> (define_insn "*baddu<mode>si3"
>   [(set (match_operand:SI 0 "register_operand" "=d")
> 	(zero_extend:SI
> 	 (match_operator:QI 1 "truncate_operator"
> 	  [(plus:GPR (match_operand:GPR 2 "register_operand" "d")
> 		     (match_operand:GPR 3 "register_operand" "d"))])))]
>   "ISA_HAS_BADDU"
>   "baddu\\t%0,%2,%3"
>   [(set_attr "type" "arith")])

Really cool and it works too!

Surprisingly I ran into a reload assert with this.  Reload wants to
unconditionally reload the argument of a subreg if it is a PLUS and we
fail badly.  I am planning to submit the reload part of the patch
separately after testing it on x86 too.

I ended up not adding the TRULY_NOOP_TRUNCATION part.  I think it's
legitimate to have (subreg:QI (plus:DI ...) <lowpart byte>) and
truncate_operator should match this (unlike my patterns which wouldn't).
 The idea being that if we can prove that the DI addition results in a
valid SI value we can just truncate with subreg.  What do you think?

Adam

	* config/mips/mips.h (ISA_HAS_BADDU): New macro.
	* config/mips/predicate.md (truncate_operator): New predicate.
	* config/mips/mips.md (*baddu<GPR:mode><GPR2:mode>): New pattern.

testsuite/
	* gcc.target/mips/octeon-baddu-1.c: New test.

Index: gcc/config/mips/mips.h
===================================================================
--- gcc.orig/config/mips/mips.h	2008-08-27 07:13:47.000000000 -0700
+++ gcc/config/mips/mips.h	2008-08-27 14:08:20.000000000 -0700
@@ -1005,6 +1005,9 @@ enum mips_code_readable_setting {
 
 /* ISA includes the pop instruction.  */
 #define ISA_HAS_POP		TARGET_OCTEON
+
+/* ISA includes the baddu instruction.  */
+#define ISA_HAS_BADDU		TARGET_OCTEON
 
 /* Add -G xx support.  */
 
Index: gcc/config/mips/predicates.md
===================================================================
--- gcc.orig/config/mips/predicates.md	2008-08-27 07:13:47.000000000 -0700
+++ gcc/config/mips/predicates.md	2008-08-27 14:10:47.000000000 -0700
@@ -285,6 +285,11 @@
 (define_predicate "order_operator"
   (match_code "lt,ltu,le,leu,ge,geu,gt,gtu"))
 
+;; Match nop and non-nop truncations.
+(define_predicate "truncate_operator"
+  (ior (and (match_code "subreg")
+	    (match_test "subreg_lowpart_p (op)"))
+       (match_code "truncate")))
 
 (define_predicate "small_data_pattern"
   (and (match_code "set,parallel,unspec,unspec_volatile,prefetch")
Index: gcc/config/mips/mips.md
===================================================================
--- gcc.orig/config/mips/mips.md	2008-08-27 07:13:47.000000000 -0700
+++ gcc/config/mips/mips.md	2008-08-27 14:08:20.000000000 -0700
@@ -1159,6 +1159,20 @@
   [(set_attr "type" "arith")
    (set_attr "mode" "SI")
    (set_attr "extended_mips16" "yes")])
+
+;; Combiner patterns for unsigned byte-add.
+
+(define_insn "*baddu<GPR:mode><GPR2:mode>"
+  [(set (match_operand:GPR2 0 "register_operand" "=d")
+	(zero_extend:GPR2
+	 (match_operator:QI 1 "truncate_operator"
+			    [(plus:GPR
+			      (match_operand:GPR 2 "register_operand" "d")
+			      (match_operand:GPR 3 "register_operand"
+						 "d"))])))]
+  "ISA_HAS_BADDU"
+  "baddu\\t%0,%2,%3"
+  [(set_attr "type" "arith")])
 
 ;;
 ;;  ....................
Index: gcc/testsuite/gcc.target/mips/octeon-baddu-1.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gcc/testsuite/gcc.target/mips/octeon-baddu-1.c	2008-08-27 07:15:11.000000000 -0700
@@ -0,0 +1,30 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O -march=octeon" } */
+/* { dg-final { scan-assembler-times "\tbaddu\t" 4 } } */
+/* { dg-final { scan-assembler-not "\tandi\t" } } */
+
+NOMIPS16 unsigned char
+g (long long a, long long b)
+{
+  return a + b;
+}
+
+NOMIPS16 unsigned long long
+h (unsigned long long a, unsigned long long b)
+{
+  unsigned char c = a + b;
+  return c;
+}
+
+NOMIPS16 long long
+ff (long long a, long long b)
+{
+  unsigned char c = a + b;
+  return c;
+}
+
+NOMIPS16 int
+gg (int a, int b)
+{
+  return (a + b) & 0xff;
+}
Index: gcc/reload.c
===================================================================
--- gcc.orig/reload.c	2008-08-27 07:13:47.000000000 -0700
+++ gcc/reload.c	2008-08-27 14:13:29.000000000 -0700
@@ -3070,7 +3070,13 @@ find_reloads (rtx insn, int replace, int
 	  /* An empty constraint or empty alternative
 	     allows anything which matched the pattern.  */
 	  if (*p == 0 || *p == ',')
-	    win = 1, badop = 0;
+	    {
+	      win = 1;
+	      badop = 0;
+	      /* Clear force_reload if above we set it incorrectly (e.g. for a
+		 match_operator operand).  */
+	      force_reload = 0;
+	    }
 
 	  /* Scan this alternative's specs for this operand;
 	     set WIN if the operand fits any letter in this alternative.