public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [patch] Fixed-point patch 8/10
@ 2007-08-02  0:20 Fu, Chao-Ying
  2007-08-04 10:14 ` Richard Sandiford
  0 siblings, 1 reply; 12+ messages in thread
From: Fu, Chao-Ying @ 2007-08-02  0:20 UTC (permalink / raw)
  To: gcc-patches; +Cc: Thekkath, Radhika, Stephens, Nigel, Mark Mitchell

[-- Attachment #1: Type: text/plain, Size: 2091 bytes --]

Hi,

  Here is the patch #8.  Please review it.  Thanks!

> 1. Merge in machine modes to support signed and unsigned
> fract and accum modes.  Handle scalar and vector modes.
DONE!
> 2. Merge in fixed-value.h and fixed-value.c to handle fixed-point values.
(fixed-value.diff)
> 3. Merge in TREE structures for fixed-point types and constants.
(tree.diff)
> 4. Merge in C front-end changes to parse _Sat, _Fract and _Accum.
(c-parser.diff)
> 5. Merge in RTL structures for fixed-point constants and operators.
(rtl.diff)
> 6. Merge in libcpp to parse fixed-point constants.
(cpp.diff)
> 7. Merge in changes to support "case" of FIXED_POINT_TYPE, FIXED_CST,
> and CONST_FIXED in .c and .h files.
(changes.diff)
> 8. Merge in the MIPS backend that supports fixed-point instructions.
(mips.diff)
> 9. Merge in configure/build system changes for the runtime library.
> 10. Merge in testsuite (from gcc.dg/fixed-point) that only run
> when the compiler is configured to enable fixed-point.

Regards,
Chao-ying

gcc/ChangeLog
2007-08-01  Chao-ying Fu  <fu@mips.com>

	* config/mips/mips.c (mips_scalar_mode_supported_p): Declare.
	(TARGET_SCALAR_MODE_SUPPORTED_P): Define.
	(mips_emit_compare): Process fixed-point modes.
	(mips_pad_arg_upward): Support fixed-point types.
	(override_options): Allow fixed-point modes in accumulators.
	(mips_pass_by_reference): Pass DQ, UDQ, DA, and UDA modes in registers.
	(mips_vector_mode_supported_p): Support V2HQmode, V2UHQmode, V2HAmode,
	V2UHAmode, V4QQmode, and V4UQQmode when TARGET_DSP.
	(mips_scalar_mode_supported_p): New function to accept fixed-point
	modes if the width is not greater than two BITS_PER_WORD.
	* config/mips/mips.h (SHORT_FRACT_TYPE_SIZE, FRACT_TYPE_SIZE,
	LONG_FRACT_TYPE_SIZE, LONG_LONG_FRACT_TYPE_SIZE,
	SHORT_ACCUM_TYPE_SIZE, ACCUM_TYPE_SIZE, LONG_ACCUM_TYPE_SIZE,
	LONG_LONG_ACCUM_TYPE_SIZE): Define.
	* config/mips/mips.md (mips-fixed.md): Include.
	* config/mips/mips-modes.def: Create VECTOR_MODES for FRACT, UFRACT,
	ACCUM, UACCUM.
	* config/mips/mips-fixed.md: New file.

[-- Attachment #2: mips.diff --]
[-- Type: application/octet-stream, Size: 11144 bytes --]

Index: gcc4x/gcc/gcc/config/mips/mips.c
===================================================================
--- gcc4x.orig/gcc/gcc/config/mips/mips.c	2007-07-30 10:17:57.000000000 -0700
+++ gcc4x/gcc/gcc/config/mips/mips.c	2007-07-30 11:05:08.000000000 -0700
@@ -410,6 +410,7 @@
 static int mips_arg_partial_bytes (CUMULATIVE_ARGS *, enum machine_mode mode,
 				   tree, bool);
 static bool mips_valid_pointer_mode (enum machine_mode);
+static bool mips_scalar_mode_supported_p (enum machine_mode);
 static bool mips_vector_mode_supported_p (enum machine_mode);
 static rtx mips_prepare_builtin_arg (enum insn_code, unsigned int, tree, unsigned int);
 static rtx mips_prepare_builtin_target (enum insn_code, unsigned int, rtx);
@@ -1313,6 +1314,9 @@
 #undef TARGET_VECTOR_MODE_SUPPORTED_P
 #define TARGET_VECTOR_MODE_SUPPORTED_P mips_vector_mode_supported_p
 
+#undef TARGET_SCALAR_MODE_SUPPORTED_P
+#define TARGET_SCALAR_MODE_SUPPORTED_P mips_scalar_mode_supported_p
+
 #undef TARGET_INIT_BUILTINS
 #define TARGET_INIT_BUILTINS mips_init_builtins
 #undef TARGET_EXPAND_BUILTIN
@@ -3473,6 +3477,15 @@
 	  *code = (invert ? EQ : NE);
 	}
     }
+  else if (ALL_FIXED_POINT_MODE_P (GET_MODE (cmp_operands[0])))
+    {
+      enum rtx_code cmp_code;
+      cmp_code = *code;
+      *code = NE;
+      *op0 = gen_rtx_REG (CCDSPmode, CCDSP_CC_REGNUM);
+      *op1 = const0_rtx;
+      mips_emit_binary (cmp_code, *op0, cmp_operands[0], cmp_operands[1]);
+    }
   else
     {
       enum rtx_code cmp_code;
@@ -4308,7 +4321,9 @@
      stack argument is passed in the last byte of the stack slot.  */
   if (type != 0
       ? INTEGRAL_TYPE_P (type) || POINTER_TYPE_P (type)
-      : GET_MODE_CLASS (mode) == MODE_INT)
+	|| FIXED_POINT_TYPE_P (type)
+      : GET_MODE_CLASS (mode) == MODE_INT
+	|| ALL_SCALAR_FIXED_POINT_MODE_P (mode))
     return false;
 
   /* Big-endian o64 pads floating-point arguments downward.  */
@@ -5380,7 +5395,7 @@
 			|| (ISA_HAS_8CC && mode == TFmode)));
 
           else if (ACC_REG_P (regno))
-	    temp = (INTEGRAL_MODE_P (mode)
+	    temp = ((INTEGRAL_MODE_P (mode) || ALL_FIXED_POINT_MODE_P (mode))
 		    && size <= UNITS_PER_WORD * 2
 		    && (size <= UNITS_PER_WORD
 			|| regno == MD_REG_FIRST
@@ -8498,7 +8513,9 @@
       int size;
 
       /* ??? How should SCmode be handled?  */
-      if (mode == DImode || mode == DFmode)
+      if (mode == DImode || mode == DFmode
+	  || mode == DQmode || mode == UDQmode
+	  || mode == DAmode || mode == UDAmode)
 	return 0;
 
       size = type ? int_size_in_bytes (type) : GET_MODE_SIZE (mode);
@@ -8760,12 +8777,30 @@
 
     case V2HImode:
     case V4QImode:
+    case V2HQmode:
+    case V2UHQmode:
+    case V2HAmode:
+    case V2UHAmode:
+    case V4QQmode:
+    case V4UQQmode:
       return TARGET_DSP;
 
     default:
       return false;
     }
 }
+
+/* Target hook for scalar_mode_supported_p.  */
+
+static bool
+mips_scalar_mode_supported_p (enum machine_mode mode)
+{
+  if (ALL_FIXED_POINT_MODE_P (mode)
+      && GET_MODE_PRECISION (mode) <=  2 * BITS_PER_WORD)
+    return true;
+
+  return default_scalar_mode_supported_p (mode);
+}
 \f
 /* If we can access small data directly (using gp-relative relocation
    operators) return the small data pointer, otherwise return null.
Index: gcc4x/gcc/gcc/config/mips/mips.h
===================================================================
--- gcc4x.orig/gcc/gcc/config/mips/mips.h	2007-07-30 10:17:57.000000000 -0700
+++ gcc4x/gcc/gcc/config/mips/mips.h	2007-07-30 11:05:08.000000000 -0700
@@ -1112,6 +1112,19 @@
 #define DOUBLE_TYPE_SIZE 64
 #define LONG_DOUBLE_TYPE_SIZE (TARGET_NEWABI ? 128 : 64)
 
+/* Define the sizes of fixed-point types.  */
+#define SHORT_FRACT_TYPE_SIZE 8
+#define FRACT_TYPE_SIZE 16
+#define LONG_FRACT_TYPE_SIZE 32
+#define LONG_LONG_FRACT_TYPE_SIZE 64
+
+#define SHORT_ACCUM_TYPE_SIZE 16
+#define ACCUM_TYPE_SIZE 32
+#define LONG_ACCUM_TYPE_SIZE 64
+/* FIXME.  LONG_LONG_ACCUM_TYPE_SIZE should be 128 bits, but GCC
+   doesn't support 128-bit integers for MIPS32 currently.  */
+#define LONG_LONG_ACCUM_TYPE_SIZE (TARGET_LONG64 ? 128 : 64)
+
 /* long double is not a fixed mode, but the idea is that, if we
    support long double, we also want a 128-bit integer type.  */
 #define MAX_FIXED_MODE_SIZE LONG_DOUBLE_TYPE_SIZE
Index: gcc4x/gcc/gcc/config/mips/mips.md
===================================================================
--- gcc4x.orig/gcc/gcc/config/mips/mips.md	2007-07-30 10:14:32.000000000 -0700
+++ gcc4x/gcc/gcc/config/mips/mips.md	2007-07-30 11:05:08.000000000 -0700
@@ -5730,3 +5730,6 @@
 ; The MIPS DSP REV 2 Instructions.
 
 (include "mips-dspr2.md")
+
+; MIPS fixed-point instructions.
+(include "mips-fixed.md")
Index: gcc4x/gcc/gcc/config/mips/mips-modes.def
===================================================================
--- gcc4x.orig/gcc/gcc/config/mips/mips-modes.def	2007-07-30 10:14:32.000000000 -0700
+++ gcc4x/gcc/gcc/config/mips/mips-modes.def	2007-07-30 11:05:08.000000000 -0700
@@ -30,6 +30,11 @@
 VECTOR_MODES (FLOAT, 8);      /*            V4HF V2SF */
 VECTOR_MODES (INT, 4);        /*            V4QI V2HI */
 
+VECTOR_MODES (FRACT, 4);	/* V4QQ  V2HQ */
+VECTOR_MODES (UFRACT, 4);	/* V4UQQ V2UHQ */
+VECTOR_MODES (ACCUM, 4);	/*       V2HA */
+VECTOR_MODES (UACCUM, 4);	/*       V2UHA */
+
 /* Paired single comparison instructions use 2 or 4 CC.  */
 CC_MODE (CCV2);
 ADJUST_BYTESIZE (CCV2, 8);
Index: gcc4x/gcc/gcc/config/mips/mips-fixed.md
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gcc4x/gcc/gcc/config/mips/mips-fixed.md	2007-07-30 11:05:08.000000000 -0700
@@ -0,0 +1,143 @@
+; This file contains MIPS instructions that support fixed-point operations.
+
+; All supported fixed-point modes
+(define_mode_macro FIXED [(QQ "") (HQ "") (SQ "") (DQ "TARGET_64BIT")
+			  (UQQ "") (UHQ "") (USQ "") (UDQ "TARGET_64BIT")
+			  (HA "") (SA "") (DA "TARGET_64BIT")
+			  (UHA "") (USA "") (UDA "TARGET_64BIT")])
+
+; For signed add/sub with saturation
+(define_mode_macro ADDSUB [(HQ "") (SQ "") (HA "") (SA "") (V2HQ "") (V2HA "")])
+(define_mode_attr addsubfmt [(HQ "ph") (SQ "w") (HA "ph") (SA "w")
+			     (V2HQ "ph") (V2HA "ph")])
+
+; For unsigned add/sub with saturation
+(define_mode_macro UADDSUB [(UQQ "TARGET_DSP") (UHQ "TARGET_DSPR2")
+			    (UHA "TARGET_DSPR2") (V4UQQ "TARGET_DSP")
+			    (V2UHQ "TARGET_DSPR2") (V2UHA "TARGET_DSPR2")])
+(define_mode_attr uaddsubfmt [(UQQ "qb") (UHQ "ph") (UHA "ph")])
+
+; For signed multiplication with saturation
+(define_mode_macro MULQ [(V2HQ "TARGET_DSP") (HQ "TARGET_DSP")
+			 (SQ "TARGET_DSPR2")])
+(define_mode_attr mulqfmt [(V2HQ "ph") (HQ "ph") (SQ "w")])
+
+; In GPR templates, a string like "<dd>subu" will expand to "subu" in the
+; 32-bit version and "dsubu" in the 64-bit version.
+(define_mode_attr dd [(QQ "") (HQ "") (SQ "") (DQ "d")
+		      (UQQ "") (UHQ "") (USQ "") (UDQ "d")
+		      (HA "") (SA "") (DA "d")
+		      (UHA "") (USA "") (UDA "d")])
+
+; The integer mode has the same size as the fixed-point mode.
+(define_mode_attr imode [(QQ "QI") (HQ "HI") (SQ "SI") (DQ "DI")
+			 (UQQ "QI") (UHQ "HI") (USQ "SI") (UDQ "DI")
+			 (HA "HI") (SA "SI") (DA "DI")
+			 (UHA "HI") (USA "SI") (UDA "DI")
+			 (V4UQQ "SI") (V2UHQ "SI") (V2UHA "SI")
+			 (V2HQ "SI") (V2HA "SI")])
+
+(define_insn "add<mode>3"
+  [(set (match_operand:FIXED 0 "register_operand" "=d")
+	(plus:FIXED (match_operand:FIXED 1 "register_operand" "d")
+		    (match_operand:FIXED 2 "register_operand" "d")))]
+  ""
+  "<dd>addu\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<imode>")])
+
+(define_insn "usadd<mode>3"
+  [(parallel
+    [(set (match_operand:UADDSUB 0 "register_operand" "=d")
+	  (us_plus:UADDSUB (match_operand:UADDSUB 1 "register_operand" "d")
+			   (match_operand:UADDSUB 2 "register_operand" "d")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+	  (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_ADDQ_S))])]
+  ""
+  "addu_s.<uaddsubfmt>\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<imode>")])
+
+(define_insn "ssadd<mode>3"
+  [(parallel
+    [(set (match_operand:ADDSUB 0 "register_operand" "=d")
+	  (ss_plus:ADDSUB (match_operand:ADDSUB 1 "register_operand" "d")
+			  (match_operand:ADDSUB 2 "register_operand" "d")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+	  (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_ADDQ_S))])]
+  "TARGET_DSP"
+  "addq_s.<addsubfmt>\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<imode>")])
+
+(define_insn "sub<mode>3"
+  [(set (match_operand:FIXED 0 "register_operand" "=d")
+        (minus:FIXED (match_operand:FIXED 1 "register_operand" "d")
+		     (match_operand:FIXED 2 "register_operand" "d")))]
+  ""
+  "<dd>subu\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<imode>")])
+
+(define_insn "ussub<mode>3"
+  [(parallel
+    [(set (match_operand:UADDSUB 0 "register_operand" "=d")
+	  (us_minus:UADDSUB (match_operand:UADDSUB 1 "register_operand" "d")
+			    (match_operand:UADDSUB 2 "register_operand" "d")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+	  (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_SUBQ_S))])]
+  ""
+  "subu_s.<uaddsubfmt>\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<imode>")])
+
+(define_insn "sssub<mode>3"
+  [(parallel
+    [(set (match_operand:ADDSUB 0 "register_operand" "=d")
+	  (ss_minus:ADDSUB (match_operand:ADDSUB 1 "register_operand" "d")
+			   (match_operand:ADDSUB 2 "register_operand" "d")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+	  (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_SUBQ_S))])]
+  "TARGET_DSP"
+  "subq_s.<addsubfmt>\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<imode>")])
+
+(define_insn "ssmul<mode>3"
+  [(parallel
+    [(set (match_operand:MULQ 0 "register_operand" "=d")
+          (ss_mult:MULQ (match_operand:MULQ 1 "register_operand" "d")
+			(match_operand:MULQ 2 "register_operand" "d")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+          (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_MULQ_RS_PH))
+     (clobber (match_scratch:DI 3 "=x"))])]
+  ""
+  "mulq_rs.<mulqfmt>\t%0,%1,%2"
+  [(set_attr "type"     "imul3")
+   (set_attr "mode"     "<imode>")])
+
+(define_insn "ssmaddsqdq4"
+  [(set (match_operand:DQ 0 "register_operand" "=a")
+        (ss_plus:DQ
+         (ss_mult:DQ (sat_fract:DQ (match_operand:SQ 1
+					"register_operand" "d"))
+                     (sat_fract:DQ (match_operand:SQ 2
+					"register_operand" "d")))
+         (match_operand:DQ 3 "register_operand" "0")))]
+  "TARGET_DSP && !TARGET_64BIT"
+  "dpaq_sa.l.w\t%q0,%1,%2"
+  [(set_attr "type" "imadd")
+   (set_attr "mode" "SI")])
+
+(define_insn "ssmsubsqdq4"
+  [(set (match_operand:DQ 0 "register_operand" "=a")
+        (ss_minus:DQ
+	 (match_operand:DQ 3 "register_operand" "0")
+         (ss_mult:DQ (sat_fract:DQ (match_operand:SQ 1
+					"register_operand" "d"))
+                     (sat_fract:DQ (match_operand:SQ 2
+					"register_operand" "d")))))]
+  "TARGET_DSP && !TARGET_64BIT"
+  "dpsq_sa.l.w\t%q0,%1,%2"
+  [(set_attr "type" "imadd")
+   (set_attr "mode" "SI")])

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] Fixed-point patch 8/10
  2007-08-02  0:20 [patch] Fixed-point patch 8/10 Fu, Chao-Ying
@ 2007-08-04 10:14 ` Richard Sandiford
  2007-08-04 10:54   ` Richard Sandiford
  2007-08-07  1:09   ` Fu, Chao-Ying
  0 siblings, 2 replies; 12+ messages in thread
From: Richard Sandiford @ 2007-08-04 10:14 UTC (permalink / raw)
  To: Fu, Chao-Ying
  Cc: gcc-patches, Thekkath, Radhika, Stephens, Nigel, Mark Mitchell

(BTW, can you generate patches with -d?  It makes them easier to review.)

"Fu, Chao-Ying" <fu@mips.com> writes:
> +  else if (ALL_FIXED_POINT_MODE_P (GET_MODE (cmp_operands[0])))
> +    {
> +      enum rtx_code cmp_code;
> +      cmp_code = *code;
> +      *code = NE;
> +      *op0 = gen_rtx_REG (CCDSPmode, CCDSP_CC_REGNUM);
> +      *op1 = const0_rtx;
> +      mips_emit_binary (cmp_code, *op0, cmp_operands[0], cmp_operands[1]);
> +    }

Simpler as:

   {
     *op0 = gen_rtx_REG (CCDSPmode, CCDSP_CC_REGNUM);
     mips_emit_binary (cmp_code, *op0, cmp_operands[0], cmp_operands[1]);
     *code = NE;
     *op1 = const0_rtx;
   }

(The FP case is different because it needs to modify cmp_code.)

>    else
>      {
>        enum rtx_code cmp_code;
> @@ -4308,7 +4321,9 @@
>       stack argument is passed in the last byte of the stack slot.  */
>    if (type != 0
>        ? INTEGRAL_TYPE_P (type) || POINTER_TYPE_P (type)
> -      : GET_MODE_CLASS (mode) == MODE_INT)
> +	|| FIXED_POINT_TYPE_P (type)
> +      : GET_MODE_CLASS (mode) == MODE_INT
> +	|| ALL_SCALAR_FIXED_POINT_MODE_P (mode))
>      return false;
>  

Formatting should be:

if (type != 0
    ? (INTEGRAL_TYPE_P (type)
       || POINTER_TYPE_P (type)
       || FIXED_POINT_TYPE_P (type))
    : (GET_MODE_CLASS (mode) == MODE_INT
       || ALL_SCALAR_FIXED_POINT_MODE_P (mode)))

(the "emacs brackets").  

> +/* Target hook for scalar_mode_supported_p.  */

Minor nit, but I'd prefer "Implement TARGET_SCALAR_MODE_SUPPORTED_P."
It's then easier to tell where one should look for documentation.

> +
> +static bool
> +mips_scalar_mode_supported_p (enum machine_mode mode)
> +{
> +  if (ALL_FIXED_POINT_MODE_P (mode)
> +      && GET_MODE_PRECISION (mode) <=  2 * BITS_PER_WORD)

Stray space.

> +/* FIXME.  LONG_LONG_ACCUM_TYPE_SIZE should be 128 bits, but GCC
> +   doesn't support 128-bit integers for MIPS32 currently.  */
> +#define LONG_LONG_ACCUM_TYPE_SIZE (TARGET_LONG64 ? 128 : 64)

Gross as it is, we actually "support" -mlong64 for MIPS32.
(As in "-mabi=eabi -mgp32 -mlong64".)  The FIXME implies that
the correct value would be 128 unconditionally, so if 32-bitness
is the problem, shouldn't we be checking TARGET_64BIT instead?

> Index: gcc4x/gcc/gcc/config/mips/mips-fixed.md
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ gcc4x/gcc/gcc/config/mips/mips-fixed.md	2007-07-30 11:05:08.000000000 -0700
> @@ -0,0 +1,143 @@
> +; This file contains MIPS instructions that support fixed-point operations.

Very minor nit, but mips.md uses ";;" for comments in column 0,
except for a few cases that have slipped by.  Let's standardise on that.

> +; In GPR templates, a string like "<dd>subu" will expand to "subu" in the
> +; 32-bit version and "dsubu" in the 64-bit version.
> +(define_mode_attr dd [(QQ "") (HQ "") (SQ "") (DQ "d")
> +		      (UQQ "") (UHQ "") (USQ "") (UDQ "d")
> +		      (HA "") (SA "") (DA "d")
> +		      (UHA "") (USA "") (UDA "d")])

FWIW, I think it'd be OK to add this to the "d" attribute in mips.md.

> +; The integer mode has the same size as the fixed-point mode.
> +(define_mode_attr imode [(QQ "QI") (HQ "HI") (SQ "SI") (DQ "DI")
> +			 (UQQ "QI") (UHQ "HI") (USQ "SI") (UDQ "DI")
> +			 (HA "HI") (SA "SI") (DA "DI")
> +			 (UHA "HI") (USA "SI") (UDA "DI")
> +			 (V4UQQ "SI") (V2UHQ "SI") (V2UHA "SI")
> +			 (V2HQ "SI") (V2HA "SI")])

This too could probably go in mips.md, as it might be useful
for other modes in future.  I think "IMODE" would be better
than "imode" for consistency with the automatic "MODE" and
"mode" attributes (and for consistency with "UNITMODE" in mips.md).

Overall, this looks very clean, thanks.

Richard

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] Fixed-point patch 8/10
  2007-08-04 10:14 ` Richard Sandiford
@ 2007-08-04 10:54   ` Richard Sandiford
  2007-08-07  1:09   ` Fu, Chao-Ying
  1 sibling, 0 replies; 12+ messages in thread
From: Richard Sandiford @ 2007-08-04 10:54 UTC (permalink / raw)
  To: Fu, Chao-Ying
  Cc: gcc-patches, Thekkath, Radhika, Stephens, Nigel, Mark Mitchell

Richard Sandiford <richard@codesourcery.com> writes:
> Simpler as:
>
>    {
>      *op0 = gen_rtx_REG (CCDSPmode, CCDSP_CC_REGNUM);
>      mips_emit_binary (cmp_code, *op0, cmp_operands[0], cmp_operands[1]);
                         ^^^^^^^^
I meant *code, sorry.

>      *code = NE;
>      *op1 = const0_rtx;
>    }

Richard

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [patch] Fixed-point patch 8/10
  2007-08-04 10:14 ` Richard Sandiford
  2007-08-04 10:54   ` Richard Sandiford
@ 2007-08-07  1:09   ` Fu, Chao-Ying
  2007-08-07  6:12     ` Richard Sandiford
  1 sibling, 1 reply; 12+ messages in thread
From: Fu, Chao-Ying @ 2007-08-07  1:09 UTC (permalink / raw)
  To: Richard Sandiford
  Cc: gcc-patches, Thekkath, Radhika, Stephens, Nigel, Mark Mitchell

[-- Attachment #1: Type: text/plain, Size: 4592 bytes --]

Richard Sandiford wrote:
> 
> (BTW, can you generate patches with -d?  It makes them easier 
> to review.)

  Ok.  I set QUILT_DIFF_OPTS to "-dp" to get the diff file.

>
> "Fu, Chao-Ying" <fu@mips.com> writes:
> > +  else if (ALL_FIXED_POINT_MODE_P (GET_MODE (cmp_operands[0])))
> > +    {
> > +      enum rtx_code cmp_code;
> > +      cmp_code = *code;
> > +      *code = NE;
> > +      *op0 = gen_rtx_REG (CCDSPmode, CCDSP_CC_REGNUM);
> > +      *op1 = const0_rtx;
> > +      mips_emit_binary (cmp_code, *op0, cmp_operands[0], 
> cmp_operands[1]);
> > +    }
> 
> Simpler as:
> 
>    {
>      *op0 = gen_rtx_REG (CCDSPmode, CCDSP_CC_REGNUM);
>      mips_emit_binary (cmp_code, *op0, cmp_operands[0], 
> cmp_operands[1]);
>      *code = NE;
>      *op1 = const0_rtx;
>    }

  Yes with your change of cmp_code to *code.

> 
> (The FP case is different because it needs to modify cmp_code.)
> 
> >    else
> >      {
> >        enum rtx_code cmp_code;
> > @@ -4308,7 +4321,9 @@
> >       stack argument is passed in the last byte of the 
> stack slot.  */
> >    if (type != 0
> >        ? INTEGRAL_TYPE_P (type) || POINTER_TYPE_P (type)
> > -      : GET_MODE_CLASS (mode) == MODE_INT)
> > +	|| FIXED_POINT_TYPE_P (type)
> > +      : GET_MODE_CLASS (mode) == MODE_INT
> > +	|| ALL_SCALAR_FIXED_POINT_MODE_P (mode))
> >      return false;
> >  
> 
> Formatting should be:
> 
> if (type != 0
>     ? (INTEGRAL_TYPE_P (type)
>        || POINTER_TYPE_P (type)
>        || FIXED_POINT_TYPE_P (type))
>     : (GET_MODE_CLASS (mode) == MODE_INT
>        || ALL_SCALAR_FIXED_POINT_MODE_P (mode)))
> 
> (the "emacs brackets").  

  Yes.  I add the brackets.

> 
> > +/* Target hook for scalar_mode_supported_p.  */
> 
> Minor nit, but I'd prefer "Implement TARGET_SCALAR_MODE_SUPPORTED_P."
> It's then easier to tell where one should look for documentation.

  Yes.  The comment is changed.

> 
> > +
> > +static bool
> > +mips_scalar_mode_supported_p (enum machine_mode mode)
> > +{
> > +  if (ALL_FIXED_POINT_MODE_P (mode)
> > +      && GET_MODE_PRECISION (mode) <=  2 * BITS_PER_WORD)
> 
> Stray space.

  Yes.  It is deleted.

> 
> > +/* FIXME.  LONG_LONG_ACCUM_TYPE_SIZE should be 128 bits, but GCC
> > +   doesn't support 128-bit integers for MIPS32 currently.  */
> > +#define LONG_LONG_ACCUM_TYPE_SIZE (TARGET_LONG64 ? 128 : 64)
> 
> Gross as it is, we actually "support" -mlong64 for MIPS32.
> (As in "-mabi=eabi -mgp32 -mlong64".)  The FIXME implies that
> the correct value would be 128 unconditionally, so if 32-bitness
> is the problem, shouldn't we be checking TARGET_64BIT instead?

  Yes.  I change it to TARGET_64BIT.

> 
> > Index: gcc4x/gcc/gcc/config/mips/mips-fixed.md
> > ===================================================================
> > --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> > +++ gcc4x/gcc/gcc/config/mips/mips-fixed.md	2007-07-30 
> 11:05:08.000000000 -0700
> > @@ -0,0 +1,143 @@
> > +; This file contains MIPS instructions that support 
> fixed-point operations.
> 
> Very minor nit, but mips.md uses ";;" for comments in column 0,
> except for a few cases that have slipped by.  Let's 
> standardise on that.

  Yes. I use ";;" for comments in mips-fixed.md.

> 
> > +; In GPR templates, a string like "<dd>subu" will expand 
> to "subu" in the
> > +; 32-bit version and "dsubu" in the 64-bit version.
> > +(define_mode_attr dd [(QQ "") (HQ "") (SQ "") (DQ "d")
> > +		      (UQQ "") (UHQ "") (USQ "") (UDQ "d")
> > +		      (HA "") (SA "") (DA "d")
> > +		      (UHA "") (USA "") (UDA "d")])
> 
> FWIW, I think it'd be OK to add this to the "d" attribute in mips.md.

  Yes.  I add this to the "d" attribute.

> 
> > +; The integer mode has the same size as the fixed-point mode.
> > +(define_mode_attr imode [(QQ "QI") (HQ "HI") (SQ "SI") (DQ "DI")
> > +			 (UQQ "QI") (UHQ "HI") (USQ "SI") (UDQ "DI")
> > +			 (HA "HI") (SA "SI") (DA "DI")
> > +			 (UHA "HI") (USA "SI") (UDA "DI")
> > +			 (V4UQQ "SI") (V2UHQ "SI") (V2UHA "SI")
> > +			 (V2HQ "SI") (V2HA "SI")])
> 
> This too could probably go in mips.md, as it might be useful
> for other modes in future.  I think "IMODE" would be better
> than "imode" for consistency with the automatic "MODE" and
> "mode" attributes (and for consistency with "UNITMODE" in mips.md).

  Yes.  I move it to mips.md and change the name to IMODE.

> 
> Overall, this looks very clean, thanks.
> 

  Thanks a lot!  The new diff file is attached.

Regards,
Chao-ying

[-- Attachment #2: mips.diff --]
[-- Type: application/octet-stream, Size: 12032 bytes --]

Index: gcc4x/gcc/gcc/config/mips/mips.c
===================================================================
--- gcc4x.orig/gcc/gcc/config/mips/mips.c	2007-08-06 17:50:08.000000000 -0700
+++ gcc4x/gcc/gcc/config/mips/mips.c	2007-08-06 17:52:10.000000000 -0700
@@ -409,6 +409,7 @@ static bool mips_callee_copies (CUMULATI
 static int mips_arg_partial_bytes (CUMULATIVE_ARGS *, enum machine_mode mode,
 				   tree, bool);
 static bool mips_valid_pointer_mode (enum machine_mode);
+static bool mips_scalar_mode_supported_p (enum machine_mode);
 static bool mips_vector_mode_supported_p (enum machine_mode);
 static rtx mips_prepare_builtin_arg (enum insn_code, unsigned int, tree, unsigned int);
 static rtx mips_prepare_builtin_target (enum insn_code, unsigned int, rtx);
@@ -1312,6 +1313,9 @@ static const unsigned char mips16e_save_
 #undef TARGET_VECTOR_MODE_SUPPORTED_P
 #define TARGET_VECTOR_MODE_SUPPORTED_P mips_vector_mode_supported_p
 
+#undef TARGET_SCALAR_MODE_SUPPORTED_P
+#define TARGET_SCALAR_MODE_SUPPORTED_P mips_scalar_mode_supported_p
+
 #undef TARGET_INIT_BUILTINS
 #define TARGET_INIT_BUILTINS mips_init_builtins
 #undef TARGET_EXPAND_BUILTIN
@@ -3472,6 +3476,13 @@ mips_emit_compare (enum rtx_code *code, 
 	  *code = (invert ? EQ : NE);
 	}
     }
+  else if (ALL_FIXED_POINT_MODE_P (GET_MODE (cmp_operands[0])))
+    {
+      *op0 = gen_rtx_REG (CCDSPmode, CCDSP_CC_REGNUM);
+      mips_emit_binary (*code, *op0, cmp_operands[0], cmp_operands[1]);
+      *code = NE;
+      *op1 = const0_rtx;
+    }
   else
     {
       enum rtx_code cmp_code;
@@ -4306,8 +4317,11 @@ mips_pad_arg_upward (enum machine_mode m
   /* Otherwise, integral types are padded downward: the last byte of a
      stack argument is passed in the last byte of the stack slot.  */
   if (type != 0
-      ? INTEGRAL_TYPE_P (type) || POINTER_TYPE_P (type)
-      : GET_MODE_CLASS (mode) == MODE_INT)
+      ? (INTEGRAL_TYPE_P (type)
+	 || POINTER_TYPE_P (type)
+	 || FIXED_POINT_TYPE_P (type))
+      : (GET_MODE_CLASS (mode) == MODE_INT
+	 || ALL_SCALAR_FIXED_POINT_MODE_P (mode)))
     return false;
 
   /* Big-endian o64 pads floating-point arguments downward.  */
@@ -5379,7 +5393,7 @@ override_options (void)
 			|| (ISA_HAS_8CC && mode == TFmode)));
 
           else if (ACC_REG_P (regno))
-	    temp = (INTEGRAL_MODE_P (mode)
+	    temp = ((INTEGRAL_MODE_P (mode) || ALL_FIXED_POINT_MODE_P (mode))
 		    && size <= UNITS_PER_WORD * 2
 		    && (size <= UNITS_PER_WORD
 			|| regno == MD_REG_FIRST
@@ -8497,7 +8511,9 @@ mips_pass_by_reference (CUMULATIVE_ARGS 
       int size;
 
       /* ??? How should SCmode be handled?  */
-      if (mode == DImode || mode == DFmode)
+      if (mode == DImode || mode == DFmode
+	  || mode == DQmode || mode == UDQmode
+	  || mode == DAmode || mode == UDAmode)
 	return 0;
 
       size = type ? int_size_in_bytes (type) : GET_MODE_SIZE (mode);
@@ -8759,12 +8775,30 @@ mips_vector_mode_supported_p (enum machi
 
     case V2HImode:
     case V4QImode:
+    case V2HQmode:
+    case V2UHQmode:
+    case V2HAmode:
+    case V2UHAmode:
+    case V4QQmode:
+    case V4UQQmode:
       return TARGET_DSP;
 
     default:
       return false;
     }
 }
+
+/* Implement TARGET_SCALAR_MODE_SUPPORTED_P.  */
+
+static bool
+mips_scalar_mode_supported_p (enum machine_mode mode)
+{
+  if (ALL_FIXED_POINT_MODE_P (mode)
+      && GET_MODE_PRECISION (mode) <= 2 * BITS_PER_WORD)
+    return true;
+
+  return default_scalar_mode_supported_p (mode);
+}
 \f
 /* If we can access small data directly (using gp-relative relocation
    operators) return the small data pointer, otherwise return null.
Index: gcc4x/gcc/gcc/config/mips/mips.h
===================================================================
--- gcc4x.orig/gcc/gcc/config/mips/mips.h	2007-08-06 17:50:08.000000000 -0700
+++ gcc4x/gcc/gcc/config/mips/mips.h	2007-08-06 17:52:11.000000000 -0700
@@ -1148,6 +1148,19 @@ extern const struct mips_rtx_cost_data *
 #define DOUBLE_TYPE_SIZE 64
 #define LONG_DOUBLE_TYPE_SIZE (TARGET_NEWABI ? 128 : 64)
 
+/* Define the sizes of fixed-point types.  */
+#define SHORT_FRACT_TYPE_SIZE 8
+#define FRACT_TYPE_SIZE 16
+#define LONG_FRACT_TYPE_SIZE 32
+#define LONG_LONG_FRACT_TYPE_SIZE 64
+
+#define SHORT_ACCUM_TYPE_SIZE 16
+#define ACCUM_TYPE_SIZE 32
+#define LONG_ACCUM_TYPE_SIZE 64
+/* FIXME.  LONG_LONG_ACCUM_TYPE_SIZE should be 128 bits, but GCC
+   doesn't support 128-bit integers for MIPS32 currently.  */
+#define LONG_LONG_ACCUM_TYPE_SIZE (TARGET_64BIT ? 128 : 64)
+
 /* long double is not a fixed mode, but the idea is that, if we
    support long double, we also want a 128-bit integer type.  */
 #define MAX_FIXED_MODE_SIZE LONG_DOUBLE_TYPE_SIZE
Index: gcc4x/gcc/gcc/config/mips/mips.md
===================================================================
--- gcc4x.orig/gcc/gcc/config/mips/mips.md	2007-08-06 17:50:08.000000000 -0700
+++ gcc4x/gcc/gcc/config/mips/mips.md	2007-08-06 17:52:11.000000000 -0700
@@ -492,7 +492,11 @@
 
 ;; In GPR templates, a string like "<d>subu" will expand to "subu" in the
 ;; 32-bit version and "dsubu" in the 64-bit version.
-(define_mode_attr d [(SI "") (DI "d")])
+(define_mode_attr d [(SI "") (DI "d")
+		     (QQ "") (HQ "") (SQ "") (DQ "d")
+		     (UQQ "") (UHQ "") (USQ "") (UDQ "d")
+		     (HA "") (SA "") (DA "d")
+		     (UHA "") (USA "") (UDA "d")])
 
 ;; This attribute gives the length suffix for a sign- or zero-extension
 ;; instruction.
@@ -525,6 +529,15 @@
 ;; floating-point mode.
 (define_mode_attr UNITMODE [(SF "SF") (DF "DF") (V2SF "SF")])
 
+;; This attribute gives the integer mode that has the same size as a
+;; fixed-point mode.
+(define_mode_attr IMODE [(QQ "QI") (HQ "HI") (SQ "SI") (DQ "DI")
+			 (UQQ "QI") (UHQ "HI") (USQ "SI") (UDQ "DI")
+			 (HA "HI") (SA "SI") (DA "DI")
+			 (UHA "HI") (USA "SI") (UDA "DI")
+			 (V4UQQ "SI") (V2UHQ "SI") (V2UHA "SI")
+			 (V2HQ "SI") (V2HA "SI")])
+
 ;; This attribute works around the early SB-1 rev2 core "F2" erratum:
 ;;
 ;; In certain cases, div.s and div.ps may have a rounding error
@@ -5729,3 +5742,6 @@
 ; The MIPS DSP REV 2 Instructions.
 
 (include "mips-dspr2.md")
+
+; MIPS fixed-point instructions.
+(include "mips-fixed.md")
Index: gcc4x/gcc/gcc/config/mips/mips-modes.def
===================================================================
--- gcc4x.orig/gcc/gcc/config/mips/mips-modes.def	2007-08-06 17:50:08.000000000 -0700
+++ gcc4x/gcc/gcc/config/mips/mips-modes.def	2007-08-06 17:52:11.000000000 -0700
@@ -29,6 +29,11 @@ FLOAT_MODE (TF, 16, mips_quad_format);
 VECTOR_MODES (FLOAT, 8);      /*            V4HF V2SF */
 VECTOR_MODES (INT, 4);        /*            V4QI V2HI */
 
+VECTOR_MODES (FRACT, 4);	/* V4QQ  V2HQ */
+VECTOR_MODES (UFRACT, 4);	/* V4UQQ V2UHQ */
+VECTOR_MODES (ACCUM, 4);	/*       V2HA */
+VECTOR_MODES (UACCUM, 4);	/*       V2UHA */
+
 /* Paired single comparison instructions use 2 or 4 CC.  */
 CC_MODE (CCV2);
 ADJUST_BYTESIZE (CCV2, 8);
Index: gcc4x/gcc/gcc/config/mips/mips-fixed.md
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gcc4x/gcc/gcc/config/mips/mips-fixed.md	2007-08-06 17:52:11.000000000 -0700
@@ -0,0 +1,128 @@
+;; This file contains MIPS instructions that support fixed-point operations.
+
+;; All supported fixed-point modes
+(define_mode_macro FIXED [(QQ "") (HQ "") (SQ "") (DQ "TARGET_64BIT")
+			  (UQQ "") (UHQ "") (USQ "") (UDQ "TARGET_64BIT")
+			  (HA "") (SA "") (DA "TARGET_64BIT")
+			  (UHA "") (USA "") (UDA "TARGET_64BIT")])
+
+;; For signed add/sub with saturation
+(define_mode_macro ADDSUB [(HQ "") (SQ "") (HA "") (SA "") (V2HQ "") (V2HA "")])
+(define_mode_attr addsubfmt [(HQ "ph") (SQ "w") (HA "ph") (SA "w")
+			     (V2HQ "ph") (V2HA "ph")])
+
+;; For unsigned add/sub with saturation
+(define_mode_macro UADDSUB [(UQQ "TARGET_DSP") (UHQ "TARGET_DSPR2")
+			    (UHA "TARGET_DSPR2") (V4UQQ "TARGET_DSP")
+			    (V2UHQ "TARGET_DSPR2") (V2UHA "TARGET_DSPR2")])
+(define_mode_attr uaddsubfmt [(UQQ "qb") (UHQ "ph") (UHA "ph")])
+
+;; For signed multiplication with saturation
+(define_mode_macro MULQ [(V2HQ "TARGET_DSP") (HQ "TARGET_DSP")
+			 (SQ "TARGET_DSPR2")])
+(define_mode_attr mulqfmt [(V2HQ "ph") (HQ "ph") (SQ "w")])
+
+(define_insn "add<mode>3"
+  [(set (match_operand:FIXED 0 "register_operand" "=d")
+	(plus:FIXED (match_operand:FIXED 1 "register_operand" "d")
+		    (match_operand:FIXED 2 "register_operand" "d")))]
+  ""
+  "<d>addu\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<IMODE>")])
+
+(define_insn "usadd<mode>3"
+  [(parallel
+    [(set (match_operand:UADDSUB 0 "register_operand" "=d")
+	  (us_plus:UADDSUB (match_operand:UADDSUB 1 "register_operand" "d")
+			   (match_operand:UADDSUB 2 "register_operand" "d")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+	  (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_ADDQ_S))])]
+  ""
+  "addu_s.<uaddsubfmt>\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<IMODE>")])
+
+(define_insn "ssadd<mode>3"
+  [(parallel
+    [(set (match_operand:ADDSUB 0 "register_operand" "=d")
+	  (ss_plus:ADDSUB (match_operand:ADDSUB 1 "register_operand" "d")
+			  (match_operand:ADDSUB 2 "register_operand" "d")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+	  (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_ADDQ_S))])]
+  "TARGET_DSP"
+  "addq_s.<addsubfmt>\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<IMODE>")])
+
+(define_insn "sub<mode>3"
+  [(set (match_operand:FIXED 0 "register_operand" "=d")
+        (minus:FIXED (match_operand:FIXED 1 "register_operand" "d")
+		     (match_operand:FIXED 2 "register_operand" "d")))]
+  ""
+  "<d>subu\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<IMODE>")])
+
+(define_insn "ussub<mode>3"
+  [(parallel
+    [(set (match_operand:UADDSUB 0 "register_operand" "=d")
+	  (us_minus:UADDSUB (match_operand:UADDSUB 1 "register_operand" "d")
+			    (match_operand:UADDSUB 2 "register_operand" "d")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+	  (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_SUBQ_S))])]
+  ""
+  "subu_s.<uaddsubfmt>\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<IMODE>")])
+
+(define_insn "sssub<mode>3"
+  [(parallel
+    [(set (match_operand:ADDSUB 0 "register_operand" "=d")
+	  (ss_minus:ADDSUB (match_operand:ADDSUB 1 "register_operand" "d")
+			   (match_operand:ADDSUB 2 "register_operand" "d")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+	  (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_SUBQ_S))])]
+  "TARGET_DSP"
+  "subq_s.<addsubfmt>\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<IMODE>")])
+
+(define_insn "ssmul<mode>3"
+  [(parallel
+    [(set (match_operand:MULQ 0 "register_operand" "=d")
+          (ss_mult:MULQ (match_operand:MULQ 1 "register_operand" "d")
+			(match_operand:MULQ 2 "register_operand" "d")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+          (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_MULQ_RS_PH))
+     (clobber (match_scratch:DI 3 "=x"))])]
+  ""
+  "mulq_rs.<mulqfmt>\t%0,%1,%2"
+  [(set_attr "type"     "imul3")
+   (set_attr "mode"     "<IMODE>")])
+
+(define_insn "ssmaddsqdq4"
+  [(set (match_operand:DQ 0 "register_operand" "=a")
+        (ss_plus:DQ
+         (ss_mult:DQ (sat_fract:DQ (match_operand:SQ 1
+					"register_operand" "d"))
+                     (sat_fract:DQ (match_operand:SQ 2
+					"register_operand" "d")))
+         (match_operand:DQ 3 "register_operand" "0")))]
+  "TARGET_DSP && !TARGET_64BIT"
+  "dpaq_sa.l.w\t%q0,%1,%2"
+  [(set_attr "type" "imadd")
+   (set_attr "mode" "SI")])
+
+(define_insn "ssmsubsqdq4"
+  [(set (match_operand:DQ 0 "register_operand" "=a")
+        (ss_minus:DQ
+	 (match_operand:DQ 3 "register_operand" "0")
+         (ss_mult:DQ (sat_fract:DQ (match_operand:SQ 1
+					"register_operand" "d"))
+                     (sat_fract:DQ (match_operand:SQ 2
+					"register_operand" "d")))))]
+  "TARGET_DSP && !TARGET_64BIT"
+  "dpsq_sa.l.w\t%q0,%1,%2"
+  [(set_attr "type" "imadd")
+   (set_attr "mode" "SI")])

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] Fixed-point patch 8/10
  2007-08-07  1:09   ` Fu, Chao-Ying
@ 2007-08-07  6:12     ` Richard Sandiford
  2007-09-06  0:06       ` Fu, Chao-Ying
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Sandiford @ 2007-08-07  6:12 UTC (permalink / raw)
  To: Fu, Chao-Ying
  Cc: gcc-patches, Thekkath, Radhika, Stephens, Nigel, Mark Mitchell

>   Thanks a lot!  The new diff file is attached.

Thanks for the updates, this looks great.  OK to install when
the time comes.

Richard

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [patch] Fixed-point patch 8/10
  2007-08-07  6:12     ` Richard Sandiford
@ 2007-09-06  0:06       ` Fu, Chao-Ying
  2007-09-10 11:50         ` Richard Sandiford
  0 siblings, 1 reply; 12+ messages in thread
From: Fu, Chao-Ying @ 2007-09-06  0:06 UTC (permalink / raw)
  To: Richard Sandiford, gcc-patches
  Cc: Mark Mitchell, Thekkath, Radhika, Stephens, Nigel

[-- Attachment #1: Type: text/plain, Size: 3810 bytes --]

Richard Sandiford wrote:
> Subject: Re: [patch] Fixed-point patch 8/10
> 
> 
> >   Thanks a lot!  The new diff file is attached.
> 
> Thanks for the updates, this looks great.  OK to install when
> the time comes.
> 
> Richard
> 

Hi, Richard,

  To improve the performance of non-saturating operations, we
add several add/sub/mul patterns in mips-fixed.md.  We try to
use saturating instructions for non-saturating operations.

Ex 1:
+;; For non-saturating unsigned vector add, we can use saturating instructions
+;; because the overflowed/underflowed result is undefined.
+(define_insn "add<mode>3"
+  [(parallel
+    [(set (match_operand:VUADDSUB 0 "register_operand" "=d")
+         (plus:VUADDSUB (match_operand:VUADDSUB 1 "register_operand" "d")
+                        (match_operand:VUADDSUB 2 "register_operand" "d")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+         (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_ADDQ_S))])]
+  ""
+  "addu_s.<uaddsubfmt>\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<IMODE>")])
+
+;; For non-saturating signed vector add, we can use saturating instructions
+;; because the overflowed/underflowed result is undefined.
+(define_insn "add<mode>3"
+  [(parallel
+    [(set (match_operand:VADDSUB 0 "register_operand" "=d")
+         (plus:VADDSUB (match_operand:VADDSUB 1 "register_operand" "d")
+                       (match_operand:VADDSUB 2 "register_operand" "d")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+         (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_ADDQ_S))])]
+  "TARGET_DSP"
+  "addq_s.<addsubfmt>\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<IMODE>")])

Ex 2:
+;; For non-saturating unsigned vector sub, we can use saturating instructions
+;; because the overflowed/underflowed result is undefined.
+(define_insn "sub<mode>3"
+  [(parallel
+    [(set (match_operand:VUADDSUB 0 "register_operand" "=d")
+         (minus:VUADDSUB (match_operand:VUADDSUB 1 "register_operand" "d")
+                         (match_operand:VUADDSUB 2 "register_operand" "d")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+         (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_SUBQ_S))])]
+  ""
+  "subu_s.<uaddsubfmt>\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<IMODE>")])
+
+;; For non-saturating signed vector sub, we can use saturating instructions
+;; because the overflowed/underflowed result is undefined.
+(define_insn "sub<mode>3"
+  [(parallel
+    [(set (match_operand:VADDSUB 0 "register_operand" "=d")
+         (minus:VADDSUB (match_operand:VADDSUB 1 "register_operand" "d")
+                        (match_operand:VADDSUB 2 "register_operand" "d")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+         (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_SUBQ_S))])]
+  "TARGET_DSP"
+  "subq_s.<addsubfmt>\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<IMODE>")])

Ex 3:
+;; For non-saturating mul, we can use saturating instructions
+;; because the overflowed/underflowed result is undefined.
+(define_insn "mul<mode>3"
+  [(parallel
+    [(set (match_operand:MULQ 0 "register_operand" "=d")
+          (mult:MULQ (match_operand:MULQ 1 "register_operand" "d")
+                    (match_operand:MULQ 2 "register_operand" "d")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+          (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_MULQ_RS_PH))
+     (clobber (match_scratch:DI 3 "=x"))])]
+  ""
+  "mulq_rs.<mulqfmt>\t%0,%1,%2"
+  [(set_attr "type"     "imul3")
+   (set_attr "mode"     "<IMODE>")])

  We also added new tests to the patch #10 to check if GCC can generate
all instructions for fixed-point types.

  Is this updated patch ok?  Thanks a lot!

Regards,
Chao-ying

[-- Attachment #2: mips.diff --]
[-- Type: application/octet-stream, Size: 17067 bytes --]

Index: gcc4x/gcc/gcc/config/mips/mips.c
===================================================================
--- gcc4x.orig/gcc/gcc/config/mips/mips.c	2007-09-04 17:13:26.000000000 -0700
+++ gcc4x/gcc/gcc/config/mips/mips.c	2007-09-04 17:14:27.000000000 -0700
@@ -408,6 +408,7 @@ static bool mips_callee_copies (CUMULATI
 static int mips_arg_partial_bytes (CUMULATIVE_ARGS *, enum machine_mode mode,
 				   tree, bool);
 static bool mips_valid_pointer_mode (enum machine_mode);
+static bool mips_scalar_mode_supported_p (enum machine_mode);
 static bool mips_vector_mode_supported_p (enum machine_mode);
 static rtx mips_prepare_builtin_arg (enum insn_code, unsigned int, tree, unsigned int);
 static rtx mips_prepare_builtin_target (enum insn_code, unsigned int, rtx);
@@ -1313,6 +1314,9 @@ static const unsigned char mips16e_save_
 #undef TARGET_VECTOR_MODE_SUPPORTED_P
 #define TARGET_VECTOR_MODE_SUPPORTED_P mips_vector_mode_supported_p
 
+#undef TARGET_SCALAR_MODE_SUPPORTED_P
+#define TARGET_SCALAR_MODE_SUPPORTED_P mips_scalar_mode_supported_p
+
 #undef TARGET_INIT_BUILTINS
 #define TARGET_INIT_BUILTINS mips_init_builtins
 #undef TARGET_EXPAND_BUILTIN
@@ -3550,6 +3554,13 @@ mips_emit_compare (enum rtx_code *code, 
 	  *code = (invert ? EQ : NE);
 	}
     }
+  else if (ALL_FIXED_POINT_MODE_P (GET_MODE (cmp_operands[0])))
+    {
+      *op0 = gen_rtx_REG (CCDSPmode, CCDSP_CC_REGNUM);
+      mips_emit_binary (*code, *op0, cmp_operands[0], cmp_operands[1]);
+      *code = NE;
+      *op1 = const0_rtx;
+    }
   else
     {
       enum rtx_code cmp_code;
@@ -4385,8 +4396,11 @@ mips_pad_arg_upward (enum machine_mode m
   /* Otherwise, integral types are padded downward: the last byte of a
      stack argument is passed in the last byte of the stack slot.  */
   if (type != 0
-      ? INTEGRAL_TYPE_P (type) || POINTER_TYPE_P (type)
-      : GET_MODE_CLASS (mode) == MODE_INT)
+      ? (INTEGRAL_TYPE_P (type)
+	 || POINTER_TYPE_P (type)
+	 || FIXED_POINT_TYPE_P (type))
+      : (GET_MODE_CLASS (mode) == MODE_INT
+	 || ALL_SCALAR_FIXED_POINT_MODE_P (mode)))
     return false;
 
   /* Big-endian o64 pads floating-point arguments downward.  */
@@ -5466,7 +5480,7 @@ override_options (void)
 			|| (ISA_HAS_8CC && mode == TFmode)));
 
           else if (ACC_REG_P (regno))
-	    temp = (INTEGRAL_MODE_P (mode)
+	    temp = ((INTEGRAL_MODE_P (mode) || ALL_FIXED_POINT_MODE_P (mode))
 		    && size <= UNITS_PER_WORD * 2
 		    && (size <= UNITS_PER_WORD
 			|| regno == MD_REG_FIRST
@@ -8612,7 +8626,9 @@ mips_pass_by_reference (CUMULATIVE_ARGS 
       int size;
 
       /* ??? How should SCmode be handled?  */
-      if (mode == DImode || mode == DFmode)
+      if (mode == DImode || mode == DFmode
+	  || mode == DQmode || mode == UDQmode
+	  || mode == DAmode || mode == UDAmode)
 	return 0;
 
       size = type ? int_size_in_bytes (type) : GET_MODE_SIZE (mode);
@@ -8874,12 +8890,30 @@ mips_vector_mode_supported_p (enum machi
 
     case V2HImode:
     case V4QImode:
+    case V2HQmode:
+    case V2UHQmode:
+    case V2HAmode:
+    case V2UHAmode:
+    case V4QQmode:
+    case V4UQQmode:
       return TARGET_DSP;
 
     default:
       return false;
     }
 }
+
+/* Implement TARGET_SCALAR_MODE_SUPPORTED_P.  */
+
+static bool
+mips_scalar_mode_supported_p (enum machine_mode mode)
+{
+  if (ALL_FIXED_POINT_MODE_P (mode)
+      && GET_MODE_PRECISION (mode) <= 2 * BITS_PER_WORD)
+    return true;
+
+  return default_scalar_mode_supported_p (mode);
+}
 \f
 /* If we can access small data directly (using gp-relative relocation
    operators) return the small data pointer, otherwise return null.
Index: gcc4x/gcc/gcc/config/mips/mips.h
===================================================================
--- gcc4x.orig/gcc/gcc/config/mips/mips.h	2007-09-04 17:13:26.000000000 -0700
+++ gcc4x/gcc/gcc/config/mips/mips.h	2007-09-04 17:14:27.000000000 -0700
@@ -1179,6 +1179,19 @@ extern enum mips_code_readable_setting m
 #define DOUBLE_TYPE_SIZE 64
 #define LONG_DOUBLE_TYPE_SIZE (TARGET_NEWABI ? 128 : 64)
 
+/* Define the sizes of fixed-point types.  */
+#define SHORT_FRACT_TYPE_SIZE 8
+#define FRACT_TYPE_SIZE 16
+#define LONG_FRACT_TYPE_SIZE 32
+#define LONG_LONG_FRACT_TYPE_SIZE 64
+
+#define SHORT_ACCUM_TYPE_SIZE 16
+#define ACCUM_TYPE_SIZE 32
+#define LONG_ACCUM_TYPE_SIZE 64
+/* FIXME.  LONG_LONG_ACCUM_TYPE_SIZE should be 128 bits, but GCC
+   doesn't support 128-bit integers for MIPS32 currently.  */
+#define LONG_LONG_ACCUM_TYPE_SIZE (TARGET_64BIT ? 128 : 64)
+
 /* long double is not a fixed mode, but the idea is that, if we
    support long double, we also want a 128-bit integer type.  */
 #define MAX_FIXED_MODE_SIZE LONG_DOUBLE_TYPE_SIZE
Index: gcc4x/gcc/gcc/config/mips/mips.md
===================================================================
--- gcc4x.orig/gcc/gcc/config/mips/mips.md	2007-09-04 17:13:26.000000000 -0700
+++ gcc4x/gcc/gcc/config/mips/mips.md	2007-09-04 17:14:27.000000000 -0700
@@ -496,7 +496,11 @@
 
 ;; In GPR templates, a string like "<d>subu" will expand to "subu" in the
 ;; 32-bit version and "dsubu" in the 64-bit version.
-(define_mode_attr d [(SI "") (DI "d")])
+(define_mode_attr d [(SI "") (DI "d")
+		     (QQ "") (HQ "") (SQ "") (DQ "d")
+		     (UQQ "") (UHQ "") (USQ "") (UDQ "d")
+		     (HA "") (SA "") (DA "d")
+		     (UHA "") (USA "") (UDA "d")])
 
 ;; This attribute gives the length suffix for a sign- or zero-extension
 ;; instruction.
@@ -529,6 +533,15 @@
 ;; floating-point mode.
 (define_mode_attr UNITMODE [(SF "SF") (DF "DF") (V2SF "SF")])
 
+;; This attribute gives the integer mode that has the same size as a
+;; fixed-point mode.
+(define_mode_attr IMODE [(QQ "QI") (HQ "HI") (SQ "SI") (DQ "DI")
+			 (UQQ "QI") (UHQ "HI") (USQ "SI") (UDQ "DI")
+			 (HA "HI") (SA "SI") (DA "DI")
+			 (UHA "HI") (USA "SI") (UDA "DI")
+			 (V4UQQ "SI") (V2UHQ "SI") (V2UHA "SI")
+			 (V2HQ "SI") (V2HA "SI")])
+
 ;; This attribute works around the early SB-1 rev2 core "F2" erratum:
 ;;
 ;; In certain cases, div.s and div.ps may have a rounding error
@@ -5985,3 +5998,6 @@
 ; The MIPS DSP REV 2 Instructions.
 
 (include "mips-dspr2.md")
+
+; MIPS fixed-point instructions.
+(include "mips-fixed.md")
Index: gcc4x/gcc/gcc/config/mips/mips-modes.def
===================================================================
--- gcc4x.orig/gcc/gcc/config/mips/mips-modes.def	2007-09-04 17:13:26.000000000 -0700
+++ gcc4x/gcc/gcc/config/mips/mips-modes.def	2007-09-04 17:14:27.000000000 -0700
@@ -29,6 +29,11 @@ FLOAT_MODE (TF, 16, mips_quad_format);
 VECTOR_MODES (FLOAT, 8);      /*            V4HF V2SF */
 VECTOR_MODES (INT, 4);        /*            V4QI V2HI */
 
+VECTOR_MODES (FRACT, 4);	/* V4QQ  V2HQ */
+VECTOR_MODES (UFRACT, 4);	/* V4UQQ V2UHQ */
+VECTOR_MODES (ACCUM, 4);	/*       V2HA */
+VECTOR_MODES (UACCUM, 4);	/*       V2UHA */
+
 /* Paired single comparison instructions use 2 or 4 CC.  */
 CC_MODE (CCV2);
 ADJUST_BYTESIZE (CCV2, 8);
Index: gcc4x/gcc/gcc/config/mips/mips-fixed.md
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gcc4x/gcc/gcc/config/mips/mips-fixed.md	2007-09-04 17:34:57.000000000 -0700
@@ -0,0 +1,252 @@
+;; This file contains MIPS instructions that support fixed-point operations.
+
+;; All supported fixed-point modes
+(define_mode_iterator FIXED [(QQ "") (HQ "") (SQ "") (DQ "TARGET_64BIT")
+			     (UQQ "") (UHQ "") (USQ "") (UDQ "TARGET_64BIT")
+			     (HA "") (SA "") (DA "TARGET_64BIT")
+			     (UHA "") (USA "") (UDA "TARGET_64BIT")])
+
+;; For signed add/sub with saturation
+(define_mode_iterator ADDSUB [(HQ "") (SQ "") (HA "") (SA "") (V2HQ "")
+			      (V2HA "")])
+;; For signed vector add/sub without saturation
+(define_mode_iterator VADDSUB [(V2HQ "") (V2HA "")])
+(define_mode_attr addsubfmt [(HQ "ph") (SQ "w") (HA "ph") (SA "w")
+			     (V2HQ "ph") (V2HA "ph")])
+
+;; For unsigned add/sub with saturation
+(define_mode_iterator UADDSUB [(UQQ "TARGET_DSP") (UHQ "TARGET_DSPR2")
+			       (UHA "TARGET_DSPR2") (V4UQQ "TARGET_DSP")
+			       (V2UHQ "TARGET_DSPR2") (V2UHA "TARGET_DSPR2")])
+;; For unsigned vector add/sub without saturation
+(define_mode_iterator VUADDSUB [(V4UQQ "TARGET_DSP") (V2UHQ "TARGET_DSPR2")
+				(V2UHA "TARGET_DSPR2")])
+(define_mode_attr uaddsubfmt [(UQQ "qb") (UHQ "ph") (UHA "ph")
+			      (V4UQQ "qb") (V2UHQ "ph") (V2UHA "ph")])
+
+;; For signed multiplication with saturation
+(define_mode_iterator MULQ [(V2HQ "TARGET_DSP") (HQ "TARGET_DSP")
+			    (SQ "TARGET_DSPR2")])
+(define_mode_attr mulqfmt [(V2HQ "ph") (HQ "ph") (SQ "w")])
+
+(define_insn "add<mode>3"
+  [(set (match_operand:FIXED 0 "register_operand" "=d")
+	(plus:FIXED (match_operand:FIXED 1 "register_operand" "d")
+		    (match_operand:FIXED 2 "register_operand" "d")))]
+  ""
+  "<d>addu\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<IMODE>")])
+
+;; For non-saturating unsigned vector add, we can use saturating instructions
+;; because the overflowed/underflowed result is undefined.
+(define_insn "add<mode>3"
+  [(parallel
+    [(set (match_operand:VUADDSUB 0 "register_operand" "=d")
+	  (plus:VUADDSUB (match_operand:VUADDSUB 1 "register_operand" "d")
+			 (match_operand:VUADDSUB 2 "register_operand" "d")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+	  (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_ADDQ_S))])]
+  ""
+  "addu_s.<uaddsubfmt>\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<IMODE>")])
+
+;; For non-saturating signed vector add, we can use saturating instructions
+;; because the overflowed/underflowed result is undefined.
+(define_insn "add<mode>3"
+  [(parallel
+    [(set (match_operand:VADDSUB 0 "register_operand" "=d")
+	  (plus:VADDSUB (match_operand:VADDSUB 1 "register_operand" "d")
+			(match_operand:VADDSUB 2 "register_operand" "d")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+	  (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_ADDQ_S))])]
+  "TARGET_DSP"
+  "addq_s.<addsubfmt>\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<IMODE>")])
+
+(define_insn "usadd<mode>3"
+  [(parallel
+    [(set (match_operand:UADDSUB 0 "register_operand" "=d")
+	  (us_plus:UADDSUB (match_operand:UADDSUB 1 "register_operand" "d")
+			   (match_operand:UADDSUB 2 "register_operand" "d")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+	  (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_ADDQ_S))])]
+  ""
+  "addu_s.<uaddsubfmt>\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<IMODE>")])
+
+(define_insn "ssadd<mode>3"
+  [(parallel
+    [(set (match_operand:ADDSUB 0 "register_operand" "=d")
+	  (ss_plus:ADDSUB (match_operand:ADDSUB 1 "register_operand" "d")
+			  (match_operand:ADDSUB 2 "register_operand" "d")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+	  (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_ADDQ_S))])]
+  "TARGET_DSP"
+  "addq_s.<addsubfmt>\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<IMODE>")])
+
+(define_insn "sub<mode>3"
+  [(set (match_operand:FIXED 0 "register_operand" "=d")
+        (minus:FIXED (match_operand:FIXED 1 "register_operand" "d")
+		     (match_operand:FIXED 2 "register_operand" "d")))]
+  ""
+  "<d>subu\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<IMODE>")])
+
+;; For non-saturating unsigned vector sub, we can use saturating instructions
+;; because the overflowed/underflowed result is undefined.
+(define_insn "sub<mode>3"
+  [(parallel
+    [(set (match_operand:VUADDSUB 0 "register_operand" "=d")
+	  (minus:VUADDSUB (match_operand:VUADDSUB 1 "register_operand" "d")
+			  (match_operand:VUADDSUB 2 "register_operand" "d")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+	  (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_SUBQ_S))])]
+  ""
+  "subu_s.<uaddsubfmt>\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<IMODE>")])
+
+;; For non-saturating signed vector sub, we can use saturating instructions
+;; because the overflowed/underflowed result is undefined.
+(define_insn "sub<mode>3"
+  [(parallel
+    [(set (match_operand:VADDSUB 0 "register_operand" "=d")
+	  (minus:VADDSUB (match_operand:VADDSUB 1 "register_operand" "d")
+			 (match_operand:VADDSUB 2 "register_operand" "d")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+	  (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_SUBQ_S))])]
+  "TARGET_DSP"
+  "subq_s.<addsubfmt>\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<IMODE>")])
+
+(define_insn "ussub<mode>3"
+  [(parallel
+    [(set (match_operand:UADDSUB 0 "register_operand" "=d")
+	  (us_minus:UADDSUB (match_operand:UADDSUB 1 "register_operand" "d")
+			    (match_operand:UADDSUB 2 "register_operand" "d")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+	  (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_SUBQ_S))])]
+  ""
+  "subu_s.<uaddsubfmt>\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<IMODE>")])
+
+(define_insn "sssub<mode>3"
+  [(parallel
+    [(set (match_operand:ADDSUB 0 "register_operand" "=d")
+	  (ss_minus:ADDSUB (match_operand:ADDSUB 1 "register_operand" "d")
+			   (match_operand:ADDSUB 2 "register_operand" "d")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+	  (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_SUBQ_S))])]
+  "TARGET_DSP"
+  "subq_s.<addsubfmt>\t%0,%1,%2"
+  [(set_attr "type" "arith")
+   (set_attr "mode" "<IMODE>")])
+
+(define_insn "ssmul<mode>3"
+  [(parallel
+    [(set (match_operand:MULQ 0 "register_operand" "=d")
+          (ss_mult:MULQ (match_operand:MULQ 1 "register_operand" "d")
+			(match_operand:MULQ 2 "register_operand" "d")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+          (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_MULQ_RS_PH))
+     (clobber (match_scratch:DI 3 "=x"))])]
+  ""
+  "mulq_rs.<mulqfmt>\t%0,%1,%2"
+  [(set_attr "type"     "imul3")
+   (set_attr "mode"     "<IMODE>")])
+
+;; For non-saturating mul, we can use saturating instructions
+;; because the overflowed/underflowed result is undefined.
+(define_insn "mul<mode>3"
+  [(parallel
+    [(set (match_operand:MULQ 0 "register_operand" "=d")
+          (mult:MULQ (match_operand:MULQ 1 "register_operand" "d")
+		     (match_operand:MULQ 2 "register_operand" "d")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+          (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_MULQ_RS_PH))
+     (clobber (match_scratch:DI 3 "=x"))])]
+  ""
+  "mulq_rs.<mulqfmt>\t%0,%1,%2"
+  [(set_attr "type"     "imul3")
+   (set_attr "mode"     "<IMODE>")])
+
+(define_insn "ssmaddsqdq4"
+  [(parallel
+    [(set (match_operand:DQ 0 "register_operand" "=a")
+	  (ss_plus:DQ
+	  (ss_mult:DQ (sat_fract:DQ (match_operand:SQ 1
+				     "register_operand" "d"))
+                      (sat_fract:DQ (match_operand:SQ 2
+				     "register_operand" "d")))
+          (match_operand:DQ 3 "register_operand" "0")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+	  (unspec:CCDSP [(match_dup 1) (match_dup 2) (match_dup 3)]
+			UNSPEC_DPAQ_SA_L_W))])]
+  "TARGET_DSP && !TARGET_64BIT"
+  "dpaq_sa.l.w\t%q0,%1,%2"
+  [(set_attr "type" "imadd")
+   (set_attr "mode" "SI")])
+
+;; For non-saturating madd, we can use saturating instructions
+;; because the overflowed/underflowed result is undefined.
+(define_insn "maddsqdq4"
+  [(parallel
+    [(set (match_operand:DQ 0 "register_operand" "=a")
+	  (plus:DQ
+	  (mult:DQ (fract_convert:DQ (match_operand:SQ 1
+				     "register_operand" "d"))
+		   (fract_convert:DQ (match_operand:SQ 2
+				      "register_operand" "d")))
+          (match_operand:DQ 3 "register_operand" "0")))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+	  (unspec:CCDSP [(match_dup 1) (match_dup 2) (match_dup 3)]
+			UNSPEC_DPAQ_SA_L_W))])]
+  "TARGET_DSP && !TARGET_64BIT"
+  "dpaq_sa.l.w\t%q0,%1,%2"
+  [(set_attr "type" "imadd")
+   (set_attr "mode" "SI")])
+
+(define_insn "ssmsubsqdq4"
+  [(parallel
+    [(set (match_operand:DQ 0 "register_operand" "=a")
+          (ss_minus:DQ
+	   (match_operand:DQ 3 "register_operand" "0")
+           (ss_mult:DQ (sat_fract:DQ (match_operand:SQ 1
+				      "register_operand" "d"))
+                       (sat_fract:DQ (match_operand:SQ 2
+				      "register_operand" "d")))))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+	  (unspec:CCDSP [(match_dup 1) (match_dup 2) (match_dup 3)]
+			UNSPEC_DPSQ_SA_L_W))])]
+  "TARGET_DSP && !TARGET_64BIT"
+  "dpsq_sa.l.w\t%q0,%1,%2"
+  [(set_attr "type" "imadd")
+   (set_attr "mode" "SI")])
+
+;; For non-saturating msub, we can use saturating instructions
+;; because the overflowed/underflowed result is undefined.
+(define_insn "msubsqdq4"
+  [(parallel
+    [(set (match_operand:DQ 0 "register_operand" "=a")
+          (minus:DQ
+	   (match_operand:DQ 3 "register_operand" "0")
+           (mult:DQ (fract_convert:DQ (match_operand:SQ 1
+				       "register_operand" "d"))
+		    (fract_convert:DQ (match_operand:SQ 2
+				       "register_operand" "d")))))
+     (set (reg:CCDSP CCDSP_OU_REGNUM)
+	  (unspec:CCDSP [(match_dup 1) (match_dup 2) (match_dup 3)]
+			UNSPEC_DPSQ_SA_L_W))])]
+  "TARGET_DSP && !TARGET_64BIT"
+  "dpsq_sa.l.w\t%q0,%1,%2"
+  [(set_attr "type" "imadd")
+   (set_attr "mode" "SI")])

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] Fixed-point patch 8/10
  2007-09-06  0:06       ` Fu, Chao-Ying
@ 2007-09-10 11:50         ` Richard Sandiford
  2007-09-10 19:18           ` Fu, Chao-Ying
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Sandiford @ 2007-09-10 11:50 UTC (permalink / raw)
  To: Fu, Chao-Ying
  Cc: gcc-patches, Mark Mitchell, Thekkath, Radhika, Stephens, Nigel

Hi Chao-Ying,

Sorry for taking so long to review this.

"Fu, Chao-Ying" <fu@mips.com> writes:
>   To improve the performance of non-saturating operations, we
> add several add/sub/mul patterns in mips-fixed.md.  We try to
> use saturating instructions for non-saturating operations.
>
> Ex 1:
> +;; For non-saturating unsigned vector add, we can use saturating instructions
> +;; because the overflowed/underflowed result is undefined.
> +(define_insn "add<mode>3"
> +  [(parallel
> +    [(set (match_operand:VUADDSUB 0 "register_operand" "=d")
> +         (plus:VUADDSUB (match_operand:VUADDSUB 1 "register_operand" "d")
> +                        (match_operand:VUADDSUB 2 "register_operand" "d")))
> +     (set (reg:CCDSP CCDSP_OU_REGNUM)
> +         (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_ADDQ_S))])]
> +  ""
> +  "addu_s.<uaddsubfmt>\t%0,%1,%2"
> +  [(set_attr "type" "arith")
> +   (set_attr "mode" "<IMODE>")])
> +
> +;; For non-saturating signed vector add, we can use saturating instructions
> +;; because the overflowed/underflowed result is undefined.
> +(define_insn "add<mode>3"
> +  [(parallel
> +    [(set (match_operand:VADDSUB 0 "register_operand" "=d")
> +         (plus:VADDSUB (match_operand:VADDSUB 1 "register_operand" "d")
> +                       (match_operand:VADDSUB 2 "register_operand" "d")))
> +     (set (reg:CCDSP CCDSP_OU_REGNUM)
> +         (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_ADDQ_S))])]
> +  "TARGET_DSP"
> +  "addq_s.<addsubfmt>\t%0,%1,%2"
> +  [(set_attr "type" "arith")
> +   (set_attr "mode" "<IMODE>")])
>
> Ex 2:
> +;; For non-saturating unsigned vector sub, we can use saturating instructions
> +;; because the overflowed/underflowed result is undefined.
> +(define_insn "sub<mode>3"
> +  [(parallel
> +    [(set (match_operand:VUADDSUB 0 "register_operand" "=d")
> +         (minus:VUADDSUB (match_operand:VUADDSUB 1 "register_operand" "d")
> +                         (match_operand:VUADDSUB 2 "register_operand" "d")))
> +     (set (reg:CCDSP CCDSP_OU_REGNUM)
> +         (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_SUBQ_S))])]
> +  ""
> +  "subu_s.<uaddsubfmt>\t%0,%1,%2"
> +  [(set_attr "type" "arith")
> +   (set_attr "mode" "<IMODE>")])
> +
> +;; For non-saturating signed vector sub, we can use saturating instructions
> +;; because the overflowed/underflowed result is undefined.
> +(define_insn "sub<mode>3"
> +  [(parallel
> +    [(set (match_operand:VADDSUB 0 "register_operand" "=d")
> +         (minus:VADDSUB (match_operand:VADDSUB 1 "register_operand" "d")
> +                        (match_operand:VADDSUB 2 "register_operand" "d")))
> +     (set (reg:CCDSP CCDSP_OU_REGNUM)
> +         (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_SUBQ_S))])]
> +  "TARGET_DSP"
> +  "subq_s.<addsubfmt>\t%0,%1,%2"
> +  [(set_attr "type" "arith")
> +   (set_attr "mode" "<IMODE>")])
>
> Ex 3:
> +;; For non-saturating mul, we can use saturating instructions
> +;; because the overflowed/underflowed result is undefined.
> +(define_insn "mul<mode>3"
> +  [(parallel
> +    [(set (match_operand:MULQ 0 "register_operand" "=d")
> +          (mult:MULQ (match_operand:MULQ 1 "register_operand" "d")
> +                    (match_operand:MULQ 2 "register_operand" "d")))
> +     (set (reg:CCDSP CCDSP_OU_REGNUM)
> +          (unspec:CCDSP [(match_dup 1) (match_dup 2)] UNSPEC_MULQ_RS_PH))
> +     (clobber (match_scratch:DI 3 "=x"))])]
> +  ""
> +  "mulq_rs.<mulqfmt>\t%0,%1,%2"
> +  [(set_attr "type"     "imul3")
> +   (set_attr "mode"     "<IMODE>")])
>
>   We also added new tests to the patch #10 to check if GCC can generate
> all instructions for fixed-point types.
>
>   Is this updated patch ok?  Thanks a lot!

My first thought was "why not do this in target-independent code?".
I.e., if the target doesn't have an addMM3 pattern for some MM for
which overflow is undefined, why not try generating ssaddMM3 or
usaddMM3 instead, before falling back on an element-wise implementation?
Presumably the trick will be useful for other targets too.  Or is the
idea that, by keeping the rtl code as a PLUS rather than [US]S_PLUS,
later optimisations might be able to take advantage of the undefinedness?

I'd like to know your thoughts and others' thoughts on that first.
If we do decide that it's the target's responsibility to duplicate
the patterns, the patch looks reasonable.

I think this is now the first uncommitted patch in the series.
FWIW, the approval for the original patch still stands, so feel
free to commit that version now if you like, and deal with this
as a follow-on.  If you'd prefer to get the extra patterns sorted
out first, that's fine too.

Richard

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [patch] Fixed-point patch 8/10
  2007-09-10 11:50         ` Richard Sandiford
@ 2007-09-10 19:18           ` Fu, Chao-Ying
  2007-09-11  4:57             ` Fu, Chao-Ying
  0 siblings, 1 reply; 12+ messages in thread
From: Fu, Chao-Ying @ 2007-09-10 19:18 UTC (permalink / raw)
  To: Richard Sandiford, gcc-patches
  Cc: Mark Mitchell, Thekkath, Radhika, Stephens, Nigel

Richard Sandiford wrote:
> 
> My first thought was "why not do this in target-independent code?".
> I.e., if the target doesn't have an addMM3 pattern for some MM for
> which overflow is undefined, why not try generating ssaddMM3 or
> usaddMM3 instead, before falling back on an element-wise 
> implementation?
> Presumably the trick will be useful for other targets too.  Or is the
> idea that, by keeping the rtl code as a PLUS rather than [US]S_PLUS,
> later optimisations might be able to take advantage of the 
> undefinedness?
> 
> I'd like to know your thoughts and others' thoughts on that first.
> If we do decide that it's the target's responsibility to duplicate
> the patterns, the patch looks reasonable.
> 
> I think this is now the first uncommitted patch in the series.
> FWIW, the approval for the original patch still stands, so feel
> free to commit that version now if you like, and deal with this
> as a follow-on.  If you'd prefer to get the extra patterns sorted
> out first, that's fine too.
> 
> 

  Ok.  I will check in the original version first.  People can discuss
and decide which way is better for the new stuff later.  Thanks!

Regards,
Chao-ying

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [patch] Fixed-point patch 8/10
  2007-09-10 19:18           ` Fu, Chao-Ying
@ 2007-09-11  4:57             ` Fu, Chao-Ying
  2007-09-11  9:25               ` Richard Sandiford
  0 siblings, 1 reply; 12+ messages in thread
From: Fu, Chao-Ying @ 2007-09-11  4:57 UTC (permalink / raw)
  To: Richard Sandiford, gcc-patches
  Cc: Mark Mitchell, Thekkath, Radhika, Stephens, Nigel

Hi,

> > 
> > My first thought was "why not do this in target-independent code?".
> > I.e., if the target doesn't have an addMM3 pattern for some MM for
> > which overflow is undefined, why not try generating ssaddMM3 or
> > usaddMM3 instead, before falling back on an element-wise 
> > implementation?
> > Presumably the trick will be useful for other targets too.  
> Or is the
> > idea that, by keeping the rtl code as a PLUS rather than [US]S_PLUS,
> > later optimisations might be able to take advantage of the 
> > undefinedness?

  Probably, but I don't know which optimization does this.
And, maybe some targets don't want to do this, due to the cost of
saturating instructions.  So, the target-independent code may need to
check the cost to see if it is beneficial.

  The advantage of using the target-dependent method is that it makes 
middle-end code simple and clean.  Each target can decide to create the
instruction patterns or not.

> > 
> > I'd like to know your thoughts and others' thoughts on that first.
> > If we do decide that it's the target's responsibility to duplicate
> > the patterns, the patch looks reasonable.
> > 
> > I think this is now the first uncommitted patch in the series.
> > FWIW, the approval for the original patch still stands, so feel
> > free to commit that version now if you like, and deal with this
> > as a follow-on.  If you'd prefer to get the extra patterns sorted
> > out first, that's fine too.
> > 
> > 
> 
>   Ok.  I will check in the original version first.  People can discuss
> and decide which way is better for the new stuff later.  Thanks!
> 

  The original version is applied.  Thanks a lot!

Regards,
Chao-ying

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] Fixed-point patch 8/10
  2007-09-11  4:57             ` Fu, Chao-Ying
@ 2007-09-11  9:25               ` Richard Sandiford
  2007-09-11 22:22                 ` Mark Mitchell
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Sandiford @ 2007-09-11  9:25 UTC (permalink / raw)
  To: Fu, Chao-Ying
  Cc: gcc-patches, Mark Mitchell, Thekkath, Radhika, Stephens, Nigel

"Fu, Chao-Ying" <fu@mips.com> writes:
>> > My first thought was "why not do this in target-independent code?".
>> > I.e., if the target doesn't have an addMM3 pattern for some MM for
>> > which overflow is undefined, why not try generating ssaddMM3 or
>> > usaddMM3 instead, before falling back on an element-wise 
>> > implementation?
>> > Presumably the trick will be useful for other targets too.  
>> Or is the
>> > idea that, by keeping the rtl code as a PLUS rather than [US]S_PLUS,
>> > later optimisations might be able to take advantage of the 
>> > undefinedness?
>
>   Probably, but I don't know which optimization does this.
> And, maybe some targets don't want to do this, due to the cost of
> saturating instructions.  So, the target-independent code may need to
> check the cost to see if it is beneficial.

But AIUI, your patch is short-circuiting the target-independent code
that decomposes a vector operation into individual elementwise operations.
Is that right?  If so, it seems very unlikely that a target would provide
a saturating vector addition that's _more_ expensive than decomposing a
vector into elements, performing addition on each pair of elements,
and reconstituting the vector.

>   The advantage of using the target-dependent method is that it makes 
> middle-end code simple and clean.  Each target can decide to create the
> instruction patterns or not.

I'm not sure I buy that argument either TBH.  There are various examples
of non-fixed-point operations that the middle-end code can expand in
different ways, depending on the capabilities of the target.  The approach
has generally been that a standard trick should be implemented once in the
middle end rather than several times across the target code.

I'm really playing Devil's Advocate here; I'm not totally opposed to
the extra patterns.

Richard

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] Fixed-point patch 8/10
  2007-09-11  9:25               ` Richard Sandiford
@ 2007-09-11 22:22                 ` Mark Mitchell
  0 siblings, 0 replies; 12+ messages in thread
From: Mark Mitchell @ 2007-09-11 22:22 UTC (permalink / raw)
  To: Fu, Chao-Ying, gcc-patches, Mark Mitchell, Thekkath, Radhika,
	Stephens, Nigel, richard

Richard Sandiford wrote:

>> And, maybe some targets don't want to do this, due to the cost of
>> saturating instructions.  So, the target-independent code may need to
>> check the cost to see if it is beneficial.
> 
> But AIUI, your patch is short-circuiting the target-independent code
> that decomposes a vector operation into individual elementwise operations.
> Is that right?  If so, it seems very unlikely that a target would provide
> a saturating vector addition that's _more_ expensive than decomposing a
> vector into elements, performing addition on each pair of elements,
> and reconstituting the vector.

FWIW, I agree with Richard here.  This seems like something that would
best be done as a target-independent optimization.  If, at some future
point, it turns out that there's a target for which this is not a win,
we can always introduce a hook.  But, just as we recently did with
ffs/clz/etc., there's a class of optimizations that are very broadly
applicable, and we might as well have that benefit on all targets.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] Fixed-point patch 8/10
       [not found] <3CB54817FDF733459B230DD27C690CEC03EE8F1B@Exchange.mips.com>
@ 2007-08-24 22:32 ` Mark Mitchell
  0 siblings, 0 replies; 12+ messages in thread
From: Mark Mitchell @ 2007-08-24 22:32 UTC (permalink / raw)
  To: Fu, Chao-Ying; +Cc: gcc-patches >> GCC Patches

Fu, Chao-Ying wrote:

>   The unreviewd patches are 7, 9 and 10.

[I've added gcc-patches to the CC list.]

> http://gcc.gnu.org/ml/gcc-patches/2007-08/msg00096.html

I have a few questions about this patch.

Why does stdfix.h #undef things before defining them?  Does the draft
standard say it should do that?

Why does convert_to_fixed special case integer_zero_node and
integer_one_node?  Why not check integer_zerop, for example?  And, do we
want fold to use FCONST[01] if it reduces something to zero too?

+      /* Check if it is a fixed-point mode to do an ordinary binary
operator. */
+      if (ALL_FIXED_POINT_MODE_P (mode))
+	goto binop;

Would you please expand on this comment?  The comment says what the code
does, but why?  I think you want something like:

  /* If this is a fixed-point operation, then we cannot use the code
below because ...  */

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2007-09-11 21:44 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-08-02  0:20 [patch] Fixed-point patch 8/10 Fu, Chao-Ying
2007-08-04 10:14 ` Richard Sandiford
2007-08-04 10:54   ` Richard Sandiford
2007-08-07  1:09   ` Fu, Chao-Ying
2007-08-07  6:12     ` Richard Sandiford
2007-09-06  0:06       ` Fu, Chao-Ying
2007-09-10 11:50         ` Richard Sandiford
2007-09-10 19:18           ` Fu, Chao-Ying
2007-09-11  4:57             ` Fu, Chao-Ying
2007-09-11  9:25               ` Richard Sandiford
2007-09-11 22:22                 ` Mark Mitchell
     [not found] <3CB54817FDF733459B230DD27C690CEC03EE8F1B@Exchange.mips.com>
2007-08-24 22:32 ` Mark Mitchell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).