RE: [MIPS] Patch for DSP REV 2 builtin functions

["Fu, Chao-Ying" <fu at mips dot com>]

Hi,

  According to your review, the new patch is attached.  Thanks a lot!

Regards,
Chao-ying

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

	* doc/extend.texi (MIPS DSP Built-in Functions): Document the DSP REV 2.
	* doc/invoke.texi (-mdspr2): Document new option.
	* config/mips/mips.md (UNSPEC_ABSQ_S_QB .. UNSPEC_DPSQX_SA_W_PH):
	New unspec for DSP REV 2.
	(<u>mulsidi3_32bit_internal): Check if !TARGET_DSPR2, because 
	these instructions are extended in DSP REV 2.
	(mips-dspr2.md): Include.
	* config/mips/mips.opt (mdspr2): New option.
	* config/mips/mips.c (mips_function_type): Add MIPS_V4QI_FTYPE_V4QI,
	MIPS_SI_FTYPE_SI_SI_SI, MIPS_DI_FTYPE_DI_USI_USI, MIPS_DI_FTYPE_SI_SI,
	MIPS_DI_FTYPE_USI_USI, MIPS_V2HI_FTYPE_SI_SI_SI.
	(override_options): Check TARGET_DSPR2 to enable MASK_DSP.
	(CODE_FOR_mips_mul_ph): Define it to CODE_FOR_mulv2hi3.
	(dsp_bdesc): Add DSP REV 2 builtins.  Remove 32-bit only DSP builtins.
	(dsp_32only_bdesc): New description table for 32-bit only DSP REV 1 and 2 builtins.
	(bdesc_map): Add one field of unsupported_target_flags.
	(bdesc_arrays):  Update entries to have extra fields.  Add dsp_32only_bdesc.
	(mips_init_builtins): Initialize new function types.
	Check unsupported_target_fileds to filter out builtins.
	* config/mips/mips.h (TARGET_CPU_CPP_BUILTINS): Define __mips_dspr2 if
	TARGET_DSPR2.
	(ASM_SPEC): Pass mdspr2 to the assembler.
	* config/mips/mips32-dspr2.md: New file.

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

	* gcc.target/mips/mips32-dspr2-type.c: New test.
	* gcc.target/mips/mips32-dspr2.c: New test.
	* gcc.target/mips/dspr2-MULT.c: New test.
	* gcc.target/mips/dspr2-MULTU.c: New test.
	* gcc.target/mips/mips32-dsp-run.c: New test to check execution.
	* gcc.target/mips/mips32-dsp.c: Change v4i8 typedef to use signed char.
	Adjust some formats.

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org
> [mailto:gcc-patches-owner@gcc.gnu.org]On Behalf Of Richard Sandiford
> Sent: Thursday, March 08, 2007 1:20 PM
> To: Fu, Chao-Ying
> Cc: gcc-patches@gcc.gnu.org; Thekkath, Radhika; Stephens, Nigel
> Subject: Re: [MIPS] Patch for DSP REV 2 builtin functions
> 
> 
> "Fu, Chao-Ying" <fu@mips.com> writes:
> >> > (define_insn "mips_dpa_w_ph"
> >> >   [(set (match_operand:DI 0 "register_operand" "=a")
> >> > 	(unspec:DI [(match_operand:DI 1 "register_operand" "0")
> >> > 		    (match_operand:V2HI 2 "reg_or_0_operand" "dYG")
> >> > 		    (match_operand:V2HI 3 "reg_or_0_operand" "dYG")]
> >> > 		   UNSPEC_DPA_W_PH))]
> >> >   "TARGET_DSPR2 && !TARGET_64BIT"
> >> >   "dpa.w.ph\t%q0,%z2,%z3"
> >> >   [(set_attr "type"	"imadd")
> >> >    (set_attr "mode"	"SI")])
> >> 
> >> What happens if you try to use the built-in functions on 
> >> 64-bit targets?
> >> Do we ICE?  Same for other such patterns.
> >
> >   Yes for the previous implementation.
> >
> >> 
> >> I suspect you need a separate table of built-in functions for the
> >> 32-bit-only built-in functions.  It's OK for those functions not to
> >> be recognised at all if TARGET_64BIT.
> >
> >   Ok.  We need one more field in "struct bdesc_map" to identify
> > unsupported target flags.  And, we need to separate the DSP REV 1
> > table as well, because some builtins are not for 64-bit 
> targets, either.
> > I merged the REV 2 table to the original dsp_bdesc, and created
> > a new table for the 32-bit only builtins.
> 
> Looks good.  And sorry for not catching this problem when the original
> DSP stuff went in.
> 
> > +Later in 2006, the MIPS DSP ASE REV 2 was introduced by adding more
> > +instructions.  The implementation of the REV 2 requires 
> the original
> > +DSP ASE architecture.  The REV 2 is enabled by the @option{-mdspr2}
> > +command-line option, which implies @option{-mdsp}.
> 
> The second sentence reads a little awkwardly to me.  How about
> something like:
> 
>     Revision 2 of the ASE was introduced in the second half of 2006.
>     This revision adds extra instructions to the original ASE, but is
>     otherwise backwards-compatible with it.  You can select revision 2
>     using the command-line option @option{-mdspr2}; this 
> option implies
>     @option{-mdsp}.
> 
> I admit that's not great either.  I'm open to better suggestions.
> 
> The documentation looks good to me otherwise.  I notice you've made
> a few corrections to the existing docs too -- thanks for that.
> 
> > @@ -10549,7 +10660,8 @@
> >    offset = 0;
> >    for (m = bdesc_arrays; m < &bdesc_arrays[ARRAY_SIZE 
> (bdesc_arrays)]; m++)
> >      {
> > -      if (m->proc == PROCESSOR_MAX || (m->proc == mips_arch))
> > +      if ((m->proc == PROCESSOR_MAX || (m->proc == mips_arch))
> > +	  && !(m->unsupported_target_flags & target_flags))
> >  	for (d = m->bdesc; d < &m->bdesc[m->size]; d++)
> >  	  if ((d->target_flags & target_flags) == d->target_flags)
> >  	    add_builtin_function (d->name, types[d->function_type],
> 
> Minor nit, but please write this as:
> 
>       if ((m->proc == PROCESSOR_MAX || m->proc == mips_arch)
> 	  && (m->unsupported_target_flags & target_flags) == 0)
> 
> OK with those changes, thanks.
> 
> Richard
>

Attachment: gcc.diff
Description: gcc.diff

Attachment: mips-dspr2.md
Description: mips-dspr2.md

Attachment: dspr2-MULT.c
Description: dspr2-MULT.c

Attachment: dspr2-MULTU.c
Description: dspr2-MULTU.c

Attachment: mips32-dspr2.c
Description: mips32-dspr2.c

Attachment: mips32-dspr2-type.c
Description: mips32-dspr2-type.c

Attachment: mips32-dsp-run.c
Description: mips32-dsp-run.c