Re: [PATCH] [MIPS] fix mips_prepend insn.

[Richard Sandiford]

Liu <pro...@gmail.com> writes:
> diff --git a/gcc/config/mips/mips-dspr2.md b/gcc/config/mips/mips-dspr2.md
> index 5ae902f..6853b9d 100644
> --- a/gcc/config/mips/mips-dspr2.md
> +++ b/gcc/config/mips/mips-dspr2.md
> @@ -345,7 +345,7 @@
>     (set_attr "mode"  "SI")])
>  
>  (define_insn "mips_prepend"
> -  [(set (match_operand:SI 0 "register_operand" "=d")
> +  [(set (match_operand:DI 0 "register_operand" "=d")
>       (unspec:SI [(match_operand:SI 1 "register_operand" "0")
>                   (match_operand:SI 2 "reg_or_0_operand" "dJ")
>                   (match_operand:SI 3 "const_int_operand" "n")]

This pattern maps directly to __builtin_mips_prepend, which is defined
to take and return an SI type, so :SI is the correct choice here.

I agree it might be nice to have a function that operates on 64-bit
values for 64-bit targets though.  For compatibility reasons,
we'd probably have to define both a new function and a new pattern
(or at least an iterator version of the current pattern).
There's currently no way of generating PREPENDD or PREPENDW either.

I consider this API to be owned by MTI, so we'd need to coordinate
with them.  Chao-Ying, what do you think?  Do you already have
something like this internally?

> @@ -353,7 +353,7 @@
>    "ISA_HAS_DSPR2"
>  {
>    if (INTVAL (operands[3]) & ~(unsigned HOST_WIDE_INT) 31)
> -    operands[2] = GEN_INT (INTVAL (operands[2]) & 31);
> +    operands[3] = GEN_INT (INTVAL (operands[3]) & 31);
>    return "prepend\t%0,%z2,%3";
>  }
>    [(set_attr "type"  "arith")

This part's obviously correct though, thanks.  Applied as below.

(Since it isn't a regression, I suppose it shouldn't strictly speaking
be applied during stage 4.  It's very isolated and fixes an obvious typo
though, so I hope no-one minds that I'm making an exception.)

Richard


gcc/
2012-02-02  Jia Liu  <pro...@gmail.com>

        * config/mips/mips-dspr2.md (mips_prepend): Mask operand 3 rather
        than operand 2.

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

Index: gcc/config/mips/mips-dspr2.md
===================================================================
--- gcc/config/mips/mips-dspr2.md       2012-02-02 18:32:00.000000000 +0000
+++ gcc/config/mips/mips-dspr2.md       2012-02-02 18:32:11.000000000 +0000
@@ -353,7 +353,7 @@ (define_insn "mips_prepend"
   "ISA_HAS_DSPR2"
 {
   if (INTVAL (operands[3]) & ~(unsigned HOST_WIDE_INT) 31)
-    operands[2] = GEN_INT (INTVAL (operands[2]) & 31);
+    operands[3] = GEN_INT (INTVAL (operands[3]) & 31);
   return "prepend\t%0,%z2,%3";
 }
   [(set_attr "type"    "arith")
Index: gcc/testsuite/gcc.target/mips/mips-prepend-1.c
===================================================================
--- /dev/null   2012-02-02 18:24:25.322640797 +0000
+++ gcc/testsuite/gcc.target/mips/mips-prepend-1.c      2012-02-02 
18:31:04.000000000 +0000
@@ -0,0 +1,8 @@
+/* { dg-options "-mdspr2" } */
+/* { dg-final { scan-assembler "prepend\[^\n\]*,10" } } */
+
+NOMIPS16 int
+foo (int x, int y)
+{
+  return __builtin_mips_prepend (x, y, 42);
+}