[patch] Handle MIPS DSP control registers
Catherine Moore
clm@codesourcery.com
Thu Oct 30 22:54:00 GMT 2008
Hi Richard,
Here is the updated patch which incorporates your comments. I will send the wwwdocs patch in a
separate message. Does this look okay to install? Thanks, Catherine
2008-10-30 Catherine Moore <clm@codesourcery.com>
* config/mips.h (DSP_CTRL_REG_FIRST): Define.
(DSP_CTRL_REG_LAST): Define.
* config/mips.c (mips_conditional_register_usage): Handle the
DSP control registers.
* doc/extend.texi
2008-10-30 Chao-ying Fu <fu@mips.com>
* gcc.target/mips/dsp-ctrl.c: New test.
Index: config/mips/mips.c
===================================================================
--- config/mips/mips.c (revision 141304)
+++ config/mips/mips.c (working copy)
@@ -13939,7 +13939,14 @@ mips_swap_registers (unsigned int i)
void
mips_conditional_register_usage (void)
{
- if (!ISA_HAS_DSP)
+
+ /* These DSP control register fields are global. */
+ if (ISA_HAS_DSP)
+ {
+ global_regs[CCDSP_PO_REGNUM] = 1;
+ global_regs[CCDSP_SC_REGNUM] = 1;
+ }
+ else
{
int regno;
Index: config/mips/mips.h
===================================================================
--- config/mips/mips.h (revision 141150)
+++ config/mips/mips.h (working copy)
@@ -1637,6 +1637,9 @@ enum mips_code_readable_setting {
#define DSP_ACC_REG_LAST 181
#define DSP_ACC_REG_NUM (DSP_ACC_REG_LAST - DSP_ACC_REG_FIRST + 1)
+#define DSP_CTRL_REG_FIRST 182
+#define DSP_CTRL_REG_LAST 187
+
#define AT_REGNUM (GP_REG_FIRST + 1)
#define HI_REGNUM (TARGET_BIG_ENDIAN ? MD_REG_FIRST : MD_REG_FIRST + 1)
#define LO_REGNUM (TARGET_BIG_ENDIAN ? MD_REG_FIRST + 1 : MD_REG_FIRST)
Index: doc/extend.texi
===================================================================
--- doc/extend.texi (revision 141150)
+++ doc/extend.texi (working copy)
@@ -8698,6 +8698,12 @@ otherwise backwards-compatible with it.
using the command-line option @option{-mdspr2}; this option implies
@option{-mdsp}.
+The SCOUNT and POS bits of the DSP control register are global. The
+WRDSP, EXTPDP, EXTPDPV and MTHLIP instructions modify the SCOUNT and
+POS bits. During optimization, the compiler will not delete these
+instructions and it will not delete calls to functions containing
+these instructions.
+
At present, GCC only provides support for operations on 32-bit
vectors. The vector type associated with 8-bit integer data is
usually called @code{v4i8}, the vector type associated with Q7
Index: testsuite/gcc.target/mips/dsp-ctrl.c
===================================================================
--- testsuite/gcc.target/mips/dsp-ctrl.c (revision 0)
+++ testsuite/gcc.target/mips/dsp-ctrl.c (revision 0)
@@ -0,0 +1,80 @@
+/* { dg-do run { target mips*-*-* } } */
+/* { dg-options "-O2" } */
+
+extern void abort (void);
+extern void exit (int);
+#if __mips_dsp
+
+void __attribute__ ((noinline))
+test1 (int i)
+{
+ __builtin_mips_wrdsp (i, 63);
+}
+
+void __attribute__ ((noinline))
+test2 ()
+{
+ long long a = 0;
+ __builtin_mips_extpdp (a, 3);
+}
+
+void __attribute__ ((noinline))
+test3 (int i)
+{
+ long long a = 0;
+ __builtin_mips_extpdp (a, i);
+}
+
+void __attribute__ ((noinline))
+test4 ()
+{
+ long long a = 0;
+ int i = 0;
+ __builtin_mips_mthlip (a, i);
+}
+
+int
+main ()
+{
+ int cntl;
+
+ /* Test 1: wrdsp */
+ __builtin_mips_wrdsp (0,63);
+ test1 (63);
+ cntl = __builtin_mips_rddsp (63);
+ if (cntl != 63)
+ abort ();
+
+ /* Test 2: extpdp */
+ __builtin_mips_wrdsp (63,63);
+ test2 ();
+ cntl = __builtin_mips_rddsp (63);
+ if (cntl != 59)
+ abort ();
+
+ /* Test 3: extpdpv */
+ __builtin_mips_wrdsp (63,63);
+ test3 (10);
+ cntl = __builtin_mips_rddsp (63);
+ if (cntl != 52)
+ abort ();
+
+ /* Test 4: mthlip */
+ __builtin_mips_wrdsp (8,63);
+ test4 ();
+ cntl = __builtin_mips_rddsp (63);
+ if (cntl != 40)
+ abort ();
+
+ exit (0);
+}
+
+#else
+
+int
+main ()
+{
+ exit (0);
+}
+
+#endif
Richard Sandiford wrote:
> Hi Chao-ying,
>
> "Chao-ying Fu" <fu@mips.com> writes:
>> Richard Sandiford wrote:
>>> Catherine Moore <clm@codesourcery.com> writes:
>>>
>>>> This patch adds definitons for the DSP control registers and then
>> handles them in registers in
>>>> mips_conditional_register_usage. Okay to install? Thanks, Catherine
>>>>
>>>>
>>>> 2008-10-22 Catherine Moore <clm@codesourcery.com>
>>>>
>>>> * config/mips.h (DSP_CTRL_REG_FIRST): Define.
>>>> (DSP_CTRL_REG_LAST): Define.
>>>> * config/mips.c (mips_conditional_register_usage): Handle the
>>>> DSP control registers.
>>>>
>>>> *** mips.c (revision 225707)
>>>> --- mips.c (local)
>>>> *************** mips_swap_registers (unsigned int i)
>>>> *** 13939,13950 ****
>>>> --- 13939,13959 ----
>>>> void
>>>> mips_conditional_register_usage (void)
>>>> {
>>>> +
>>>> + /* These DSP control register fields are global. */
>>>> + if (ISA_HAS_DSP)
>>>> + {
>>>> + global_regs[CCDSP_PO_REGNUM] = 1;
>>>> + global_regs[CCDSP_SC_REGNUM] = 1;
>>>> + }
>>>> if (!ISA_HAS_DSP)
>>>> {
>>>> int regno;
>>>>
>>>> for (regno = DSP_ACC_REG_FIRST; regno <= DSP_ACC_REG_LAST;
>> regno++)
>>>> fixed_regs[regno] = call_used_regs[regno] = 1;
>>>> + for (regno = DSP_CTRL_REG_FIRST; regno <= DSP_CTRL_REG_LAST;
>> regno++)
>>>> + fixed_regs[regno] = call_used_regs[regno] = 1;
>>>> }
>>>> if (!TARGET_HARD_FLOAT)
>>>> {
>>>> *** mips.h (revision 225707)
>>>> --- mips.h (local)
>>>> *************** enum mips_code_readable_setting {
>>>> *** 1637,1642 ****
>>>> --- 1637,1645 ----
>>>> #define DSP_ACC_REG_LAST 181
>>>> #define DSP_ACC_REG_NUM (DSP_ACC_REG_LAST - DSP_ACC_REG_FIRST + 1)
>>>>
>>>> + #define DSP_CTRL_REG_FIRST 182
>>>> + #define DSP_CTRL_REG_LAST 187
>>>> +
>>>> #define AT_REGNUM (GP_REG_FIRST + 1)
>>>> #define HI_REGNUM (TARGET_BIG_ENDIAN ? MD_REG_FIRST : MD_REG_FIRST +
>> 1)
>>>> #define LO_REGNUM (TARGET_BIG_ENDIAN ? MD_REG_FIRST + 1 :
>> MD_REG_FIRST)
>>> I'm not saying the patch is wrong, but I'm not sure what the
>>> motivation is. What goes wrong if we don't fix these registers?
>>> And why do we want to make the SCOUNT and POS fields global?
>>> Why aren't those two fields call-clobbered like the others?
>>>
>>> This would be an ABI change from 4.3, so would need to be mentioned
>>> somewhere. The DSP part of extend.texi should also mention which
>>> fields are global.
>>>
>>> Richard
>>>
>> Back in Nov 16, 2005, Nigel Stephens, David Ung, Radhika Thekkath, and I
>> discussed
>> the usage of DSP control registers and had the conclusion as follows.
>> -----
>> 1. Function calls are assumed to clobber all fields of
>> the DSP control register. The compiler must make sure that
>> DSP instructions that read any field of the DSP
>> control register are not moved before or after function calls.
>>
>> 2. SCOUNT and POS of the DSP control register are global.
>> The compiler cannot delete instructions that modify
>> SCOUNT or POS in any situation. These instructions
>> include WRDSP, EXTPDP, EXTPDPV, and MTHLIP.
>> Also, the compiler cannot delete any function calls
>> that jump to a function containing WRDSP, EXTPDP,
>> EXTPDPV, or MTHLIP.
>>
>> 3. Based on 1 and 2, programmers must know that after a function
>> call, intrinsics that depend on SCOUNT and POS can be used.
>> But, there is no guarantee that four other fields (CCOND, OUFLAG, EFI, C)
>> of the DSP control register are valid, so programmers
>> should re-initialize the values of (CCOND, OUFLAG, EFI, C)
>> if these fields are needed by following intrinsics.
>> -----
>>
>> One test was created to cover the behavior of DSP control registers.
>
> OK, thanks for the info.
>
> (It's a little unfortunate that the ABI was changed four months after
> the DSP support was added to FSF GCC, yet it seems to have taken three
> years for this decision to reach an FSF forum. But there's nothing
> to be done about that now.)
>
> As far as the patch itself goes: the DSP registers are already
> (unconditionally) fixed and call-clobbered, so the second part of the
> patch should be redundant. (The registers should be fixed because
> they aren't allocatable even when DSP support is enabled. And as
> Chao-ying's message says, they're defined to be call-clobbered.)
>
> As I said above, I think we should document the globalness of SCOUNT
> and POS in the DSP part of extend.texi. And because we're changing
> the ABI, I think this deserves a mention in the "Caveats" section of
> gcc-4.4/changes.html.
>
> We should take the test Chao-Ying posted and add it to gcc.target/mips.
> (I believe it's covered by MTI's copyright assignment.) I think it
> would be better to run the test with -O2 and mark the subroutines as
> "noinline"; that should stress more RTL code without compromising
> the test.
>
> Very minor nit, sorry, but I'd prefer if (ISA_HAS_DSP) ...; else ...;
> over if (ISA_HAS_DSP) ...; if (!ISA_HAS_DSP) ...;
>
> Richard
More information about the Gcc-patches
mailing list