From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gcc-patches-return-409766-listarch-gcc-patches=gcc.gnu.org@gcc.gnu.org>
Received: (qmail 501 invoked by alias); 9 Oct 2015 14:45:11 -0000
Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-patches.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-patches/>
List-Post: <mailto:gcc-patches@gcc.gnu.org>
List-Help: <mailto:gcc-patches-help@gcc.gnu.org>
Sender: gcc-patches-owner@gcc.gnu.org
Received: (qmail 482 invoked by uid 89); 9 Oct 2015 14:45:10 -0000
Authentication-Results: sourceware.org; auth=none
X-Virus-Found: No
X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL,BAYES_05,RCVD_IN_DNSWL_LOW,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2
X-HELO: mailapp01.imgtec.com
Received: from mailapp01.imgtec.com (HELO mailapp01.imgtec.com) (195.59.15.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 09 Oct 2015 14:44:59 +0000
Received: from KLMAIL01.kl.imgtec.org (unknown [192.168.5.35])	by Websense Email Security Gateway with ESMTPS id 9FBD7B4DB8E4E;	Fri,  9 Oct 2015 15:44:51 +0100 (IST)
Received: from LEMAIL01.le.imgtec.org (192.168.152.62) by KLMAIL01.kl.imgtec.org (192.168.5.35) with Microsoft SMTP Server (TLS) id 14.3.195.1; Fri, 9 Oct 2015 15:44:54 +0100
Received: from LEMAIL01.le.imgtec.org ([fe80::5ae:ee16:f4b9:cda9]) by LEMAIL01.le.imgtec.org ([fe80::5ae:ee16:f4b9:cda9%17]) with mapi id 14.03.0210.002; Fri, 9 Oct 2015 15:44:54 +0100
From: Matthew Fortune <Matthew.Fortune@imgtec.com>
To: Robert Suchanek <Robert.Suchanek@imgtec.com>,	"'Catherine_Moore@mentor.com'" <Catherine_Moore@mentor.com>
CC: "'gcc-patches@gcc.gnu.org'" <gcc-patches@gcc.gnu.org>
Subject: RE: [PATCH 1/4] [MIPS] Add support for MIPS SIMD Architecture (MSA)
Date: Fri, 09 Oct 2015 14:45:00 -0000
Message-ID: <6D39441BF12EF246A7ABCE6654B0235361C4726A@LEMAIL01.le.imgtec.org>
References: <B5E67142681B53468FAF6B7C31356562441AF59F@hhmail02.hh.imgtec.org> <6D39441BF12EF246A7ABCE6654B023532126F93C@LEMAIL01.le.imgtec.org>
In-Reply-To: <6D39441BF12EF246A7ABCE6654B023532126F93C@LEMAIL01.le.imgtec.org>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-IsSubscribed: yes
X-SW-Source: 2015-10/txt/msg00987.txt.bz2

Hi Robert,

Next batch of comments. This set covers the rest of mips-msa.md.

>+++ b/gcc/config/mips/mips-msa.md
>+(define_expand "vec_perm<mode>"
>+  [(match_operand:MSA 0 "register_operand")
>+   (match_operand:MSA 1 "register_operand")
>+   (match_operand:MSA 2 "register_operand")
>+   (match_operand:<VIMODE> 3 "register_operand")]
>+  "ISA_HAS_MSA"
>+{
>+  /* The optab semantics are that index 0 selects the first element
>+     of operands[1] and the highest index selects the last element
>+     of operands[2].  This is the oppossite order from "vshf.df wd,rs,wt"
>+     where index 0 selects the first element of wt and the highest index
>+     selects the last element of ws.  We therefore swap the operands here=
.  */
>+  emit_insn (gen_msa_vshf<mode> (operands[0], operands[3], operands[2],
>+				 operands[1]));
>+  DONE;
>+})

Can you make this the real instruction instead of msa_vshf and give it a
proper pattern (vec_select, vec_concat) etc. Swap the builtin to target
this pattern and swap the operands for the builtin expansion in C code
like you have done for some other patterns.

>+(define_expand "neg<mode>2"
>+  [(match_operand:IMSA 0 "register_operand")
>+   (match_operand:IMSA 1 "register_operand")]
>+  "ISA_HAS_MSA"
>+{
>+  rtx reg =3D gen_reg_rtx (<MODE>mode);
>+  emit_insn (gen_msa_ldi<mode> (reg, const0_rtx));
>+  emit_insn (gen_sub<mode>3 (operands[0], reg, operands[1]));
>+  DONE;
>+})
>+
>+(define_expand "neg<mode>2"
>+  [(match_operand:FMSA 0 "register_operand")
>+   (match_operand:FMSA 1 "register_operand")]
>+  "ISA_HAS_MSA"
>+{
>+  rtx reg =3D gen_reg_rtx (<MODE>mode);
>+  emit_move_insn (reg, CONST0_RTX (<MODE>mode));
>+  emit_insn (gen_sub<mode>3 (operands[0], reg, operands[1]));
>+  DONE;
>+})

Can't these two collapse into one like this?

(define_expand "neg<mode>2"
  [(set (match_operand:MSA 0 "register_operand")
        (minus:MSA (match_dup 2)
                   (match_operand:MSA 1 "register_operand")))]
  "ISA_HAS_MSA"
{
  operands[2] =3D CONST0_RTX (<MODE>mode);
})

I'd hope the const_vector then gets emitted as an LDI? I haven't
checked that there is a pattern for using LDI for FP const_vector
moves.

>+(define_expand "msa_ldi<mode>"
>+  [(match_operand:IMSA 0 "register_operand")
>+   (match_operand 1 "const_imm10_operand")]
>+  "ISA_HAS_MSA"
>+{
>+  unsigned n_elts =3D GET_MODE_NUNITS (<MODE>mode);
>+  rtvec v =3D rtvec_alloc (n_elts);
>+  HOST_WIDE_INT val =3D INTVAL (operands[1]);
>+  unsigned int i;
>+
>+  if (<MODE>mode !=3D V16QImode)
>+    {
>+      unsigned shift =3D HOST_BITS_PER_WIDE_INT - 10;
>+      val =3D trunc_int_for_mode ((val << shift) >> shift, <UNITMODE>mode=
);
>+    }
>+  else
>+    val =3D trunc_int_for_mode (val, <UNITMODE>mode);
>+
>+  for (i =3D 0; i < n_elts; i++)
>+    RTVEC_ELT (v, i) =3D GEN_INT (val);
>+  emit_move_insn (operands[0],
>+		  gen_rtx_CONST_VECTOR (<MODE>mode, v));
>+  DONE;
>+})

This is really weird. We shouldn't be simply discarding bits that don't fit.
This needs to accept all immediates and generate the correct code to
get a replicated constant of that value into a register. I think it is
probably OK to trunc_int_for_mode on the original 'val' for the
<UNIT>mode but anything out of range for V*HI/SI/DI needs to be expanded
properly.

Please do not gen_msa_ldi anywhere other than from MSA builtins. There is
no need just emit a move directly.

>+(define_insn "msa_vshf<mode>"
>+  [(set (match_operand:MSA 0 "register_operand" "=3Df")
>+	(unspec:MSA [(match_operand:<VIMODE> 1 "register_operand" "0")
>+		     (match_operand:MSA 2 "register_operand" "f")
>+		     (match_operand:MSA 3 "register_operand" "f")]
>+		    UNSPEC_MSA_VSHF))]
>+  "ISA_HAS_MSA"
>+  "vshf.<msafmt>\t%w0,%w2,%w3"
>+  [(set_attr "type" "simd_sld")
>+   (set_attr "mode" "<MODE>")])

Delete this and switch to using vec_perm directly instead for the builtin.

>+;; 128bit MSA modes only in msa registers or memory.  An exception is all=
owing

128-bit MSA modes can only exist in MSA registers or memory. ...

>+;; Offset load
>+(define_expand "msa_ld_<msafmt_f>"
>+  [(match_operand:MSA 0 "register_operand")
>+   (match_operand 1 "pmode_register_operand")
>+   (match_operand 2 "aq10<msafmt>_operand")]
>+  "ISA_HAS_MSA"
>+{
>+  rtx addr =3D plus_constant (GET_MODE (operands[1]), operands[1],
>+				      INTVAL (operands[2]));
>+  mips_emit_move (operands[0], gen_rtx_MEM (<MODE>mode, addr));
>+  DONE;
>+})
>+
>+;; Offset store
>+(define_expand "msa_st_<msafmt_f>"
>+  [(match_operand:MSA 0 "register_operand")
>+   (match_operand 1 "pmode_register_operand")
>+   (match_operand 2 "aq10<msafmt>_operand")]
>+  "ISA_HAS_MSA"
>+{
>+  rtx addr =3D plus_constant (GET_MODE (operands[1]), operands[1],
>+			    INTVAL (operands[2]));
>+  mips_emit_move (gen_rtx_MEM (<MODE>mode, addr), operands[0]);
>+  DONE;
>+})

There's no real need to expand these in C code. The patterns can be used
to create the RTL. As an aside, I don't really see the point in intrinsics
to load and store data the same thing can be done from straight C.=20
The patterns also can't be used for const or volatile data as their
builtin prototypes are neither. I suspect they should at least support
pointers to const data.

>+;; Integer operations
>+(define_insn "add<mode>3"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df,f,f")
>+	(plus:IMSA
>+	  (match_operand:IMSA 1 "register_operand" "f,f,f")
>+	  (match_operand:IMSA 2 "reg_or_vector_same_ximm5_operand" "f,Unv5,Uuv5"=
)))]
>+  "ISA_HAS_MSA"
>+{
>+  switch (which_alternative)
>+    {
>+    case 0:
>+      return "addv.<msafmt>\t%w0,%w1,%w2";
>+    case 1:
>+      {
>+	HOST_WIDE_INT val =3D INTVAL (CONST_VECTOR_ELT (operands[2], 0));
>+
>+	operands[2] =3D GEN_INT (-val);
>+	return "subvi.<msafmt>\t%w0,%w1,%d2";
>+      }
>+    case 2:
>+      {
>+	HOST_WIDE_INT val =3D INTVAL (CONST_VECTOR_ELT (operands[2], 0));
>+
>+	operands[2] =3D GEN_INT (val);
>+	return "addvi.<msafmt>\t%w0,%w1,%d2";

This can use operands[2] unchanged, just use %E2 instead.

>+      }
>+    default:
>+      gcc_unreachable ();
>+    }
>+}
>+  [(set_attr "alu_type" "simd_add")
>+   (set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "sub<mode>3"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df,f,f")
>+	(minus:IMSA
>+	  (match_operand:IMSA 1 "register_operand" "f,f,f")
>+	  (match_operand:IMSA 2 "reg_or_vector_same_ximm5_operand" "f,Unv5,Uuv5"=
)))]
>+  "ISA_HAS_MSA"
>+{
>+  switch (which_alternative)
>+    {
>+    case 0:
>+      return "subv.<msafmt>\t%w0,%w1,%w2";
>+    case 1:
>+      {
>+	HOST_WIDE_INT val =3D INTVAL (CONST_VECTOR_ELT (operands[2], 0));
>+
>+	operands[2] =3D GEN_INT (-val);
>+	return "addvi.<msafmt>\t%w0,%w1,%d2";
>+      }
>+    case 2:
>+      {
>+	HOST_WIDE_INT val =3D INTVAL (CONST_VECTOR_ELT (operands[2], 0));
>+
>+	operands[2] =3D GEN_INT (val);
>+	return "subvi.<msafmt>\t%w0,%w1,%d2";
>+      }
>+    default:
>+      gcc_unreachable ();
>+    }
>+}
>+  [(set_attr "alu_type" "simd_add")
>+   (set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])

I don't believe we need to handle constants for the sub<mode>3 pattern if
we have them covered by add<mode>3. I can't see any canonicalisation rule
in rtl.texi to say this though but simplify_plus_minus seems to show this
to be true.

>+(define_insn "xorv16qi3"
>+  [(set (match_operand:V16QI 0 "register_operand" "=3Df,f")
>+	(xor:V16QI
>+	  (match_operand:V16QI 1 "register_operand" "f,f")
>+	  (match_operand:V16QI 2 "reg_or_vector_same_byte_operand" "f,Ubv8")))]
>+  "ISA_HAS_MSA"
>+{
>+  if (which_alternative =3D=3D 1)
>+    {
>+      operands[2] =3D CONST_VECTOR_ELT (operands[2], 0);
>+      return "xori.b\t%w0,%w1,%B2";

I don't think you need to use %B2 it is already validated as a replicated
constant vector and in range so %E should be OK? This also means that you
don't need to do the output patterns in C code and can just use
"@
 alt0
 alt1"
=09=09=09=09
xori.b seems like it could be useful for other modes too when the immediate
has replicated bit patterns across all bytes of the element size.

>+    }
>+  else
>+    return "xor.v\t%w0,%w1,%w2";
>+}
>+  [(set_attr "type" "simd_logic")
>+   (set_attr "mode" "TI,V16QI")])
>+
>+(define_insn "xor<mode>3"
>+  [(set (match_operand:IMSA_DWH 0 "register_operand" "=3Df,f")
>+	(xor:IMSA_DWH
>+	  (match_operand:IMSA_DWH 1 "register_operand" "f,f")
>+	  (match_operand:IMSA_DWH 2 "reg_or_vector_same_<mode>_set_operand" "f,Y=
C")))]
>+  "ISA_HAS_MSA"
>+{
>+  if (which_alternative =3D=3D 1)
>+    {
>+      HOST_WIDE_INT val =3D INTVAL (CONST_VECTOR_ELT (operands[2], 0));
>+      int vlog2 =3D exact_log2 (val);
>+      gcc_assert (vlog2 !=3D -1);
>+      operands[2] =3D GEN_INT (vlog2);
>+      return "bnegi.%v0\t%w0,%w1,%2";

If this pattern occurs frequently then this should switch to a formatter an=
d be
handled in print_operand.

>+    }
>+  else
>+    return "xor.v\t%w0,%w1,%w2";
>+}
>+  [(set_attr "type" "simd_logic,simd_bit")
>+   (set_attr "mode" "TI,<MODE>")])
>+
>+(define_insn "iorv16qi3"
>+  [(set (match_operand:V16QI 0 "register_operand" "=3Df,f")
>+	(ior:V16QI
>+	  (match_operand:V16QI 1 "register_operand" "f,f")
>+	  (match_operand:V16QI 2 "reg_or_vector_same_byte_operand" "f,Ubv8")))]
>+  "ISA_HAS_MSA"
>+{
>+  if (which_alternative =3D=3D 1)
>+    {
>+      operands[2] =3D CONST_VECTOR_ELT (operands[2], 0);
>+      return "ori.b\t%w0,%w1,%B2";

Switch to %E and "@ output pattern syntax.

>+    }
>+  else
>+    return "or.v\t%w0,%w1,%w2";
>+}
>+  [(set_attr "type" "simd_logic")
>+   (set_attr "mode" "TI,V16QI")])
>+
>+(define_insn "andv16qi3"
>+  [(set (match_operand:V16QI 0 "register_operand" "=3Df,f")
>+	(and:V16QI
>+	  (match_operand:V16QI 1 "register_operand" "f,f")
>+	  (match_operand:V16QI 2 "reg_or_vector_same_byte_operand" "f,Ubv8")))]
>+  "ISA_HAS_MSA"
>+{
>+  if (which_alternative =3D=3D 1)
>+    {
>+      operands[2] =3D CONST_VECTOR_ELT (operands[2], 0);
>+      return "andi.b\t%w0,%w1,%B2";

Likewise.

>+    }
>+  else
>+    return "and.v\t%w0,%w1,%w2";
>+}
>+  [(set_attr "type" "simd_logic")
>+   (set_attr "mode" "TI,V16QI")])
>+
>+(define_insn "vlshr<mode>3"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df,f")
>+	(lshiftrt:IMSA
>+	  (match_operand:IMSA 1 "register_operand" "f,f")
>+	  (match_operand:IMSA 2 "reg_or_vector_same_uimm6_operand" "f,Uuv6")))]
>+  "ISA_HAS_MSA"
>+{
>+  if (which_alternative =3D=3D 0)
>+    return "srl.<msafmt>\t%w0,%w1,%w2";
>+
>+  operands[2] =3D GEN_INT (INTVAL (CONST_VECTOR_ELT (operands[2], 0))
>+			 & <shift_mask>);

Is this here because this pattern is used to expand builtins? It would be
nice if something other than the output pattern had cleaned this up already
and issued a warning about shift overflow. Does this match what happens when
shifting by wider than a scalar type.

>+  return "srli.<msafmt>\t%w0,%w1,%2";
>+}
>+  [(set_attr "type" "simd_shift")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "vashr<mode>3"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df,f")
>+	(ashiftrt:IMSA
>+	  (match_operand:IMSA 1 "register_operand" "f,f")
>+	  (match_operand:IMSA 2 "reg_or_vector_same_uimm6_operand" "f,Uuv6")))]
>+  "ISA_HAS_MSA"
>+{
>+  if (which_alternative =3D=3D 0)
>+    return "sra.<msafmt>\t%w0,%w1,%w2";
>+
>+  operands[2] =3D GEN_INT (INTVAL (CONST_VECTOR_ELT (operands[2], 0))
>+			 & <shift_mask>);

likewise.

>+  return "srai.<msafmt>\t%w0,%w1,%2";
>+}
>+  [(set_attr "type" "simd_shift")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "vashl<mode>3"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df,f")
>+	(ashift:IMSA
>+	  (match_operand:IMSA 1 "register_operand" "f,f")
>+	  (match_operand:IMSA 2 "reg_or_vector_same_uimm6_operand" "f,Uuv6")))]
>+  "ISA_HAS_MSA"
>+{
>+  if (which_alternative =3D=3D 0)
>+    return "sll.<msafmt>\t%w0,%w1,%w2";
>+
>+  operands[2] =3D GEN_INT (INTVAL (CONST_VECTOR_ELT (operands[2], 0))
>+			 & <shift_mask>);

likewise.

>+  return "slli.<msafmt>\t%w0,%w1,%2";
>+}
>+  [(set_attr "type" "simd_shift")
>+   (set_attr "mode" "<MODE>")])
>+

...

>+(define_insn "msa_fmadd_<msafmt>"
>+  [(set (match_operand:FMSA 0 "register_operand" "=3Df")
>+	(plus:FMSA (mult:FMSA (match_operand:FMSA 2 "register_operand" "f")
>+			      (match_operand:FMSA 3 "register_operand" "f"))
>+		   (match_operand:FMSA 1 "register_operand" "0")))]
>+  "ISA_HAS_MSA"
>+  "fmadd.<msafmt>\t%w0,%w2,%w3"
>+  [(set_attr "type" "simd_fmadd")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_fmsub_<msafmt>"
>+  [(set (match_operand:FMSA 0 "register_operand" "=3Df")
>+	(minus:FMSA (match_operand:FMSA 1 "register_operand" "0")
>+		    (mult:FMSA (match_operand:FMSA 2 "register_operand" "f")
>+			       (match_operand:FMSA 3 "register_operand" "f"))))]
>+  "ISA_HAS_MSA"
>+  "fmsub.<msafmt>\t%w0,%w2,%w3"
>+  [(set_attr "type" "simd_fmadd")
>+   (set_attr "mode" "<MODE>")])

These need to be usable from the fma*4 pattern(s) as well.

>+;; Built-in functions
>+(define_insn "msa_add_a_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(plus:IMSA (abs:IMSA (match_operand:IMSA 1 "register_operand" "f"))
>+		   (abs:IMSA (match_operand:IMSA 2 "register_operand" "f"))))]
>+  "ISA_HAS_MSA"
>+  "add_a.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_adds_a_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(ss_plus:IMSA
>+	  (abs:IMSA (match_operand:IMSA 1 "register_operand" "f"))
>+	  (abs:IMSA (match_operand:IMSA 2 "register_operand" "f"))))]
>+  "ISA_HAS_MSA"
>+  "adds_a.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])

This can be follow on work but we should be including fixed point vector
types for these I think. This applies to the following patterns
too.

>+
>+(define_insn "ssadd<mode>3"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(ss_plus:IMSA (match_operand:IMSA 1 "register_operand" "f")
>+		      (match_operand:IMSA 2 "register_operand" "f")))]
>+  "ISA_HAS_MSA"
>+  "adds_s.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "usadd<mode>3"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(us_plus:IMSA (match_operand:IMSA 1 "register_operand" "f")
>+		      (match_operand:IMSA 2 "register_operand" "f")))]
>+  "ISA_HAS_MSA"
>+  "adds_u.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])
>+

>+(define_expand "msa_addvi_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand")
>+	(plus:IMSA (match_operand:IMSA 1 "register_operand")
>+		   (match_operand 2 "const_uimm5_operand")))]
>+  "ISA_HAS_MSA"
>+{
>+  unsigned n_elts =3D GET_MODE_NUNITS (<MODE>mode);
>+  rtvec v =3D rtvec_alloc (n_elts);
>+  HOST_WIDE_INT val =3D INTVAL (operands[2]);
>+  unsigned int i;
>+
>+  for (i =3D 0; i < n_elts; i++)
>+    RTVEC_ELT (v, i) =3D GEN_INT (val);
>+
>+  emit_insn (gen_msa_addvi_<msafmt>_insn (operands[0], operands[1],
>+					  gen_rtx_CONST_VECTOR (<MODE>mode, v)));
>+  DONE;
>+})

This pattern is too common. Please expand this in the builtin expand
C code.=20

>+
>+(define_expand "msa_andi_b"
>+  [(set (match_operand:V16QI 0 "register_operand")
>+	(and:V16QI (match_operand:V16QI 1 "register_operand")
>+		   (match_operand:QI 2 "const_uimm8_operand")))]
>+  "ISA_HAS_MSA"
>+{
>+  rtvec v =3D rtvec_alloc (16);
>+  HOST_WIDE_INT val =3D INTVAL (operands[2]);
>+  unsigned int i;
>+
>+  for (i =3D 0; i < 16; i++)
>+    RTVEC_ELT (v, i) =3D GEN_INT (val);
>+
>+  emit_insn (gen_msa_andi_b_insn (operands[0], operands[1],
>+				  gen_rtx_CONST_VECTOR (V16QImode, v)));
>+  DONE;
>+})

likewise.

>+
>+(define_insn "msa_addvi_<msafmt>_insn"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(plus:IMSA
>+	  (match_operand:IMSA 1 "register_operand" "f")
>+	  (match_operand:IMSA 2 "const_vector_same_uimm5_operand" "")))]
>+  "ISA_HAS_MSA"
>+  "addvi.<msafmt>\t%w0,%w1,%E2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])

Why isn't this pattern part of the general msa add pattern which should
support both constants and registers as operand 2? It is also missing
a constraint for the immediate.

>+
>+(define_insn "msa_andi_b_insn"
>+  [(set (match_operand:V16QI 0 "register_operand" "=3Df")
>+	(and:V16QI
>+	  (match_operand:V16QI 1 "register_operand" "f")
>+	  (match_operand:V16QI 2 "const_vector_same_uimm8_operand" "")))]

Likewise.

>+  "ISA_HAS_MSA"
>+  "andi.b\t%w0,%w1,%E2"
>+  [(set_attr "type" "simd_logic")
>+   (set_attr "mode" "V16QI")])
>+

END USEFUL commnets

>+(define_insn "msa_bclr_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "f")
>+		      (match_operand:IMSA 2 "register_operand" "f")]
>+		     UNSPEC_MSA_BCLR))]
>+  "ISA_HAS_MSA"
>+  "bclr.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_bit")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_bclri_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "f")
>+		      (match_operand 2 "const_<bitimm>_operand" "")]
>+		     UNSPEC_MSA_BCLRI))]
>+  "ISA_HAS_MSA"
>+  "bclri.<msafmt>\t%w0,%w1,%2"
>+  [(set_attr "type" "simd_bit")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_binsl_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "0")
>+		      (match_operand:IMSA 2 "register_operand" "f")
>+		      (match_operand:IMSA 3 "register_operand" "f")]
>+		     UNSPEC_MSA_BINSL))]
>+  "ISA_HAS_MSA"
>+  "binsl.<msafmt>\t%w0,%w2,%w3"
>+  [(set_attr "type" "simd_bitins")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_binsli_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "0")
>+		      (match_operand:IMSA 2 "register_operand" "f")
>+		      (match_operand 3 "const_<bitimm>_operand" "")]
>+		     UNSPEC_MSA_BINSLI))]
>+  "ISA_HAS_MSA"
>+  "binsli.<msafmt>\t%w0,%w2,%3"
>+  [(set_attr "type" "simd_bitins")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_binsr_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "0")
>+		      (match_operand:IMSA 2 "register_operand" "f")
>+		      (match_operand:IMSA 3 "register_operand" "f")]
>+		     UNSPEC_MSA_BINSR))]
>+  "ISA_HAS_MSA"
>+  "binsr.<msafmt>\t%w0,%w2,%w3"
>+  [(set_attr "type" "simd_bitins")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_binsri_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "0")
>+		      (match_operand:IMSA 2 "register_operand" "f")
>+		      (match_operand 3 "const_<bitimm>_operand" "")]
>+		     UNSPEC_MSA_BINSRI))]
>+  "ISA_HAS_MSA"
>+  "binsri.<msafmt>\t%w0,%w2,%3"
>+  [(set_attr "type" "simd_bitins")
>+   (set_attr "mode" "<MODE>")])

As a follow up these instructions may be represenatble with standard
RTL.

>+(define_insn "msa_bmnz_v_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "0")
>+		      (match_operand:IMSA 2 "register_operand" "f")
>+		      (match_operand:IMSA 3 "register_operand" "f")]
>+		     UNSPEC_MSA_BMNZ_V))]
>+  "ISA_HAS_MSA"
>+  "bmnz.v\t%w0,%w2,%w3"
>+  [(set_attr "type" "simd_bitmov")
>+   (set_attr "mode" "TI")])

This is representable in RTL. AND, IOR, NOT etc.

>+
>+(define_insn "msa_bmnzi_b"
>+  [(set (match_operand:V16QI 0 "register_operand" "=3Df")
>+	(unspec:V16QI [(match_operand:V16QI 1 "register_operand" "0")
>+		       (match_operand:V16QI 2 "register_operand" "f")
>+		       (match_operand 3 "const_uimm8_operand" "")]
>+		      UNSPEC_MSA_BMNZI_B))]
>+  "ISA_HAS_MSA"
>+  "bmnzi.b\t%w0,%w2,%3"
>+  [(set_attr "type" "simd_bitmov")
>+   (set_attr "mode" "V16QI")])
>+
>+(define_insn "msa_bmz_v_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "0")
>+		      (match_operand:IMSA 2 "register_operand" "f")
>+		      (match_operand:IMSA 3 "register_operand" "f")]
>+		     UNSPEC_MSA_BMZ_V))]
>+  "ISA_HAS_MSA"
>+  "bmz.v\t%w0,%w2,%w3"
>+  [(set_attr "type" "simd_bitmov")
>+   (set_attr "mode" "TI")])
>+
>+(define_insn "msa_bmzi_b"
>+  [(set (match_operand:V16QI 0 "register_operand" "=3Df")
>+	(unspec:V16QI [(match_operand:V16QI 1 "register_operand" "0")
>+		       (match_operand:V16QI 2 "register_operand" "f")
>+		       (match_operand 3 "const_uimm8_operand" "")]
>+		      UNSPEC_MSA_BMZI_B))]
>+  "ISA_HAS_MSA"
>+  "bmzi.b\t%w0,%w2,%3"
>+  [(set_attr "type" "simd_bitmov")
>+   (set_attr "mode" "V16QI")])

Likewise to here.

>+(define_insn "msa_bneg_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "f")
>+		      (match_operand:IMSA 2 "register_operand" "f")]
>+		     UNSPEC_MSA_BNEG))]
>+  "ISA_HAS_MSA"
>+  "bneg.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_bit")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_bnegi_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "f")
>+		       (match_operand 2 "const_msa_branch_operand" "")]
>+		     UNSPEC_MSA_BNEGI))]
>+  "ISA_HAS_MSA"
>+  "bnegi.<msafmt>\t%w0,%w1,%2"
>+  [(set_attr "type" "simd_bit")
>+   (set_attr "mode" "<MODE>")])

Can't see any way to represent these at the moment but like all the
unspecs they should be revisited to see if they can be targetted.

>+
>+(define_insn "msa_bsel_v_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "0")
>+		      (match_operand:IMSA 2 "register_operand" "f")
>+		      (match_operand:IMSA 3 "register_operand" "f")]
>+		     UNSPEC_MSA_BSEL_V))]
>+  "ISA_HAS_MSA"
>+  "bsel.v\t%w0,%w2,%w3"
>+  [(set_attr "type" "simd_bitmov")
>+   (set_attr "mode" "TI")])

This can also be standard RTL.

>+
>+(define_insn "msa_bseli_b"
>+  [(set (match_operand:V16QI 0 "register_operand" "=3Df")
>+	(unspec:V16QI [(match_operand:V16QI 1 "register_operand" "0")
>+		       (match_operand:V16QI 2 "register_operand" "f")
>+		       (match_operand 3 "const_uimm8_operand" "")]
>+		      UNSPEC_MSA_BSELI_B))]
>+  "ISA_HAS_MSA"
>+  "bseli.b\t%w0,%w2,%3"
>+  [(set_attr "type" "simd_bitmov")
>+   (set_attr "mode" "V16QI")])

Likewise.

>+(define_insn "msa_bset_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "f")
>+		      (match_operand:IMSA 2 "register_operand" "f")]
>+		     UNSPEC_MSA_BSET))]
>+  "ISA_HAS_MSA"
>+  "bset.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_bit")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_bseti_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "f")
>+		      (match_operand 2 "const_<bitimm>_operand" "")]
>+		     UNSPEC_MSA_BSETI))]
>+  "ISA_HAS_MSA"
>+  "bseti.<msafmt>\t%w0,%w1,%2"
>+  [(set_attr "type" "simd_bit")
>+   (set_attr "mode" "<MODE>")])

Can't see how to do these in standard RTL for now.

For the builtins that accept immediates what happens when a user
provides an out-of-range constant? It would be good to have
warnings rather than ICEs.

>+(define_code_iterator ICC [eq le leu lt ltu])
>+
>+(define_code_attr icc
>+    [(eq  "eq")
>+     (le  "le_s")
>+     (leu "le_u")
>+     (lt  "lt_s")
>+     (ltu "lt_u")])
>+
>+(define_code_attr icci
>+    [(eq  "eqi")
>+     (le  "lei_s")
>+     (leu "lei_u")
>+     (lt  "lti_s")
>+     (ltu "lti_u")])
>+
>+(define_code_attr cmpi
>+    [(eq   "s")
>+     (le   "s")
>+     (leu  "u")
>+     (lt   "s")
>+     (ltu  "u")])
>+
>+(define_insn "msa_c<ICC:icc>_<IMSA:msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(ICC:IMSA (match_operand:IMSA 1 "register_operand" "f")
>+		  (match_operand:IMSA 2 "register_operand" "f")))]
>+  "ISA_HAS_MSA"
>+  "c<ICC:icc>.<IMSA:msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_c<ICC:icci>i_<IMSA:msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(ICC:IMSA
>+	  (match_operand:IMSA 1 "register_operand" "f")
>+	  (match_operand:IMSA 2 "const_vector_same_cmp<ICC:cmpi>imm4_operand" ""=
)))]
>+  "ISA_HAS_MSA"
>+  "c<ICC:icci>.<IMSA:msafmt>\t%w0,%w1,%E2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])

Can't these be combined together with two alternatives?

>+
>+(define_insn "msa_c<ICC:icci>_<IMSA:msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(ICC:IMSA (match_operand:IMSA 1 "register_operand" "f")
>+				(match_operand 2 "const_imm5_operand" ""))]
>+		     UNSPEC_MSA_CMPI))]
>+  "ISA_HAS_MSA"
>+  "c<ICC:icci>.<IMSA:msafmt>\t%w0,%w1,%2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])

Do we need this separate instruction? It should be expanded to have a
replicated constant vector for operand 2 and just use the pattern above.

>+(define_insn "msa_dotp_s_<msafmt>"
>+  [(set (match_operand:IMSA_DWH 0 "register_operand" "=3Df")
>+	(unspec:IMSA_DWH [(match_operand:<VHMODE> 1 "register_operand" "f")
>+			  (match_operand:<VHMODE> 2 "register_operand" "f")]
>+			 UNSPEC_MSA_DOTP_S))]
>+  "ISA_HAS_MSA"
>+  "dotp_s.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_mul")
>+   (set_attr "mode" "<MODE>")])

We 'could' do this in standard RTL. Whether it would ever match is question=
able.

>+(define_insn "msa_dotp_u_<msafmt>"
>+  [(set (match_operand:IMSA_DWH 0 "register_operand" "=3Df")
>+	(unspec:IMSA_DWH [(match_operand:<VHMODE> 1 "register_operand" "f")
>+			  (match_operand:<VHMODE> 2 "register_operand" "f")]
>+			 UNSPEC_MSA_DOTP_U))]
>+  "ISA_HAS_MSA"
>+  "dotp_u.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_mul")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_dpadd_s_<msafmt>"
>+  [(set (match_operand:IMSA_DWH 0 "register_operand" "=3Df")
>+	(unspec:IMSA_DWH [(match_operand:IMSA_DWH 1 "register_operand" "0")
>+			  (match_operand:<VHMODE> 2 "register_operand" "f")
>+			  (match_operand:<VHMODE> 3 "register_operand" "f")]
>+			 UNSPEC_MSA_DPADD_S))]
>+  "ISA_HAS_MSA"
>+  "dpadd_s.<msafmt>\t%w0,%w2,%w3"
>+  [(set_attr "type" "simd_mul")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_dpadd_u_<msafmt>"
>+  [(set (match_operand:IMSA_DWH 0 "register_operand" "=3Df")
>+	(unspec:IMSA_DWH [(match_operand:IMSA_DWH 1 "register_operand" "0")
>+			  (match_operand:<VHMODE> 2 "register_operand" "f")
>+			  (match_operand:<VHMODE> 3 "register_operand" "f")]
>+			 UNSPEC_MSA_DPADD_U))]
>+  "ISA_HAS_MSA"
>+  "dpadd_u.<msafmt>\t%w0,%w2,%w3"
>+  [(set_attr "type" "simd_mul")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_dpsub_s_<msafmt>"
>+  [(set (match_operand:IMSA_DWH 0 "register_operand" "=3Df")
>+	(unspec:IMSA_DWH [(match_operand:IMSA_DWH 1 "register_operand" "0")
>+			  (match_operand:<VHMODE> 2 "register_operand" "f")
>+			  (match_operand:<VHMODE> 3 "register_operand" "f")]
>+			 UNSPEC_MSA_DPSUB_S))]
>+  "ISA_HAS_MSA"
>+  "dpsub_s.<msafmt>\t%w0,%w2,%w3"
>+  [(set_attr "type" "simd_mul")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_dpsub_u_<msafmt>"
>+  [(set (match_operand:IMSA_DWH 0 "register_operand" "=3Df")
>+	(unspec:IMSA_DWH [(match_operand:IMSA_DWH 1 "register_operand" "0")
>+			  (match_operand:<VHMODE> 2 "register_operand" "f")
>+			  (match_operand:<VHMODE> 3 "register_operand" "f")]
>+			 UNSPEC_MSA_DPSUB_U))]
>+  "ISA_HAS_MSA"
>+  "dpsub_u.<msafmt>\t%w0,%w2,%w3"
>+  [(set_attr "type" "simd_mul")
>+   (set_attr "mode" "<MODE>")])

Likewise to here.

>+
>+(define_insn "msa_fclass_<msafmt>"
>+  [(set (match_operand:<VIMODE> 0 "register_operand" "=3Df")
>+	(unspec:<VIMODE> [(match_operand:FMSA 1 "register_operand" "f")]
>+			 UNSPEC_MSA_FCLASS))]
>+  "ISA_HAS_MSA"
>+  "fclass.<msafmt>\t%w0,%w1"
>+  [(set_attr "type" "simd_fclass")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_fcaf_<msafmt>"
>+  [(set (match_operand:<VIMODE> 0 "register_operand" "=3Df")
>+	(unspec:<VIMODE> [(match_operand:FMSA 1 "register_operand" "f")
>+			  (match_operand:FMSA 2 "register_operand" "f")]
>+			 UNSPEC_MSA_FCAF))]
>+  "ISA_HAS_MSA"
>+  "fcaf.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_fcmp")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_fcune_<FMSA:msafmt>"
>+  [(set (match_operand:<VIMODE> 0 "register_operand" "=3Df")
>+	(unspec:<VIMODE> [(match_operand:FMSA 1 "register_operand" "f")
>+			  (match_operand:FMSA 2 "register_operand" "f")]
>+			 UNSPEC_MSA_FCUNE))]
>+  "ISA_HAS_MSA"
>+  "fcune.<FMSA:msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_fcmp")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_code_iterator FCC [unordered ordered eq ne le lt uneq unle unlt])
>+
>+(define_code_attr fcc
>+    [(unordered "fcun")
>+     (ordered   "fcor")
>+     (eq        "fceq")
>+     (ne        "fcne")
>+     (uneq      "fcueq")
>+     (unle      "fcule")
>+     (unlt      "fcult")
>+     (le        "fcle")
>+     (lt        "fclt")])
>+
>+(define_int_iterator FSC_UNS [UNSPEC_MSA_FSAF UNSPEC_MSA_FSUN UNSPEC_MSA_=
FSOR
>+			      UNSPEC_MSA_FSEQ UNSPEC_MSA_FSNE UNSPEC_MSA_FSUEQ
>+			      UNSPEC_MSA_FSUNE UNSPEC_MSA_FSULE UNSPEC_MSA_FSULT
>+			      UNSPEC_MSA_FSLE UNSPEC_MSA_FSLT])
>+
>+(define_int_attr fsc
>+    [(UNSPEC_MSA_FSAF  "fsaf")
>+     (UNSPEC_MSA_FSUN  "fsun")
>+     (UNSPEC_MSA_FSOR  "fsor")
>+     (UNSPEC_MSA_FSEQ  "fseq")
>+     (UNSPEC_MSA_FSNE  "fsne")
>+     (UNSPEC_MSA_FSUEQ "fsueq")
>+     (UNSPEC_MSA_FSUNE "fsune")
>+     (UNSPEC_MSA_FSULE "fsule")
>+     (UNSPEC_MSA_FSULT "fsult")
>+     (UNSPEC_MSA_FSLE  "fsle")
>+     (UNSPEC_MSA_FSLT  "fslt")])
>+
>+(define_insn "msa_<FCC:fcc>_<FMSA:msafmt>"
>+  [(set (match_operand:<VIMODE> 0 "register_operand" "=3Df")
>+	(FCC:<VIMODE> (match_operand:FMSA 1 "register_operand" "f")
>+		      (match_operand:FMSA 2 "register_operand" "f")))]
>+  "ISA_HAS_MSA"
>+  "<FCC:fcc>.<FMSA:msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_fcmp")
>+   (set_attr "mode" "<MODE>")])

Can't the msa builtins target these patterns directly? Follow on work should
implement "mov<mode>cc" for vector modes.

>+
>+(define_insn "msa_<fsc>_<FMSA:msafmt>"
>+  [(set (match_operand:<VIMODE> 0 "register_operand" "=3Df")
>+	(unspec:<VIMODE> [(match_operand:FMSA 1 "register_operand" "f")
>+			   (match_operand:FMSA 2 "register_operand" "f")]
>+			 FSC_UNS))]
>+  "ISA_HAS_MSA"
>+  "<fsc>.<FMSA:msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_fcmp")
>+   (set_attr "mode" "<MODE>")])

i.e. this is not necessary.

>+(define_mode_attr FINT
>+   [(V4SF "V4SI")
>+    (V2DF "V2DI")])
>+
>+(define_mode_attr fint
>+   [(V4SF "v4si")
>+    (V2DF "v2di")])
>+
>+(define_mode_attr FQ
>+   [(V4SF "V8HI")
>+    (V2DF "V4SI")])
>+
>+(define_mode_attr FINTCNV
>+  [(V4SF "I2S")
>+   (V2DF "I2D")])
>+
>+(define_mode_attr FINTCNV_2
>+  [(V4SF "S2I")
>+   (V2DF "D2I")])
>+
>+(define_insn "float<fint><FMSA:mode>2"
>+  [(set (match_operand:FMSA 0 "register_operand" "=3Df")
>+	(float:FMSA (match_operand:<FINT> 1 "register_operand" "f")))]
>+  "ISA_HAS_MSA"
>+  "ffint_s.<msafmt>\t%w0,%w1"
>+  [(set_attr "type" "simd_fcvt")
>+   (set_attr "cnv_mode" "<FINTCNV>")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "floatuns<fint><FMSA:mode>2"
>+  [(set (match_operand:FMSA 0 "register_operand" "=3Df")
>+	(unsigned_float:FMSA (match_operand:<FINT> 1 "register_operand" "f")))]
>+  "ISA_HAS_MSA"
>+  "ffint_u.<msafmt>\t%w0,%w1"
>+  [(set_attr "type" "simd_fcvt")
>+   (set_attr "cnv_mode" "<FINTCNV>")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_mode_attr FFQ
>+  [(V4SF "V8HI")
>+   (V2DF "V4SI")])
>+
>+(define_insn "msa_ffql_<msafmt>"
>+  [(set (match_operand:FMSA 0 "register_operand" "=3Df")
>+	(unspec:FMSA [(match_operand:<FQ> 1 "register_operand" "f")]
>+		     UNSPEC_MSA_FFQL))]
>+  "ISA_HAS_MSA"
>+  "ffql.<msafmt>\t%w0,%w1"
>+  [(set_attr "type" "simd_fcvt")
>+   (set_attr "cnv_mode" "<FINTCNV>")
>+   (set_attr "mode" "<MODE>")])

There are fixed point vector modes in GCC. Perhaps improve fixed point supp=
ort
in follow on work.

>+(define_insn "msa_ffqr_<msafmt>"
>+  [(set (match_operand:FMSA 0 "register_operand" "=3Df")
>+	(unspec:FMSA [(match_operand:<FQ> 1 "register_operand" "f")]
>+		     UNSPEC_MSA_FFQR))]
>+  "ISA_HAS_MSA"
>+  "ffqr.<msafmt>\t%w0,%w1"
>+  [(set_attr "type" "simd_fcvt")
>+   (set_attr "cnv_mode" "<FINTCNV>")
>+   (set_attr "mode" "<MODE>")])

Likewise.

>+
>+;; Note used directly by builtins but via the following define_expand.
>+(define_insn "msa_fill_<msafmt>_insn"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(vec_duplicate:IMSA
>+	  (match_operand:<UNITMODE> 1 "reg_or_0_operand" "dJ")))]
>+  "ISA_HAS_MSA"
>+  "fill.<msafmt>\t%w0,%z1"
>+  [(set_attr "type" "simd_fill")
>+   (set_attr "mode" "<MODE>")])
>+
>+;; Expand builtin for HImode and QImode which takes SImode.
>+(define_expand "msa_fill_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand")
>+	(vec_duplicate:IMSA
>+	  (match_operand:<RES> 1 "reg_or_0_operand")))]
>+  "ISA_HAS_MSA"
>+{
>+  if ((GET_MODE_SIZE (<UNITMODE>mode) < GET_MODE_SIZE (<RES>mode))
>+      && (REG_P (operands[1]) || (GET_CODE (operands[1]) =3D=3D SUBREG
>+				  && REG_P (SUBREG_REG (operands[1])))))
>+    operands[1] =3D lowpart_subreg (<UNITMODE>mode, operands[1], <RES>mod=
e);
>+  emit_insn (gen_msa_fill_<msafmt>_insn (operands[0], operands[1]));
>+  DONE;
>+})

Let's not do this. Just change the builtin prototypes instead and have
them match naturally.

>+(define_insn "msa_fill_<msafmt_f>"
>+  [(set (match_operand:FMSA 0 "register_operand" "=3Df")
>+	(vec_duplicate:FMSA
>+	  (match_operand:<UNITMODE> 1 "reg_or_0_operand" "dJ")))]
>+  "ISA_HAS_MSA"
>+  "fill.<msafmt>\t%w0,%z1"
>+  [(set_attr "type" "simd_fill")
>+   (set_attr "mode" "<MODE>")])

Separate out the zero alternative and use LDI. I'm not certain but I
think we should be emitting '#' for the V2DI and V2DF mode non-zero
cases, for safety if nothing else.

>+;; Note that fill.d and fill.d_f will be split later if !TARGET_64BIT.
>+(define_split
>+  [(set (match_operand:V2DI 0 "register_operand")
>+	(vec_duplicate:V2DI
>+	  (match_operand:DI 1 "reg_or_0_operand")))]
>+  "reload_completed && TARGET_MSA && !TARGET_64BIT"
>+  [(const_int 0)]
>+{
>+  mips_split_msa_fill_d (operands[0], operands[1]);
>+  DONE;
>+})

The 'or_0' should be unnecessary here.

>+(define_insn "smax<mode>3"
>+  [(set (match_operand:FMSA 0 "register_operand" "=3Df")
>+	(smax:FMSA (match_operand:FMSA 1 "register_operand" "f")
>+		   (match_operand:FMSA 2 "register_operand" "f")))]
>+  "ISA_HAS_MSA"
>+  "fmax.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_fminmax")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "umax<mode>3"
>+  [(set (match_operand:FMSA 0 "register_operand" "=3Df")
>+	(umax:FMSA (match_operand:FMSA 1 "register_operand" "f")
>+		   (match_operand:FMSA 2 "register_operand" "f")))]
>+  "ISA_HAS_MSA"
>+  "fmax_a.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_fminmax")
>+   (set_attr "mode" "<MODE>")])

Are you sure the semantics of fmax_a and umax match? I.e. is umax
supposed to use the magitude of an FP value and ignore the sign
bit. It seems a bit odd to me.

>+(define_insn "smin<mode>3"
>+  [(set (match_operand:FMSA 0 "register_operand" "=3Df")
>+	(smin:FMSA (match_operand:FMSA 1 "register_operand" "f")
>+		   (match_operand:FMSA 2 "register_operand" "f")))]
>+  "ISA_HAS_MSA"
>+  "fmin.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_fminmax")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "umin<mode>3"
>+  [(set (match_operand:FMSA 0 "register_operand" "=3Df")
>+	(umin:FMSA (match_operand:FMSA 1 "register_operand" "f")
>+		   (match_operand:FMSA 2 "register_operand" "f")))]
>+  "ISA_HAS_MSA"
>+  "fmin_a.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_fminmax")
>+   (set_attr "mode" "<MODE>")])

Likewise.

>+
>+(define_insn "msa_frcp_<msafmt>"
>+  [(set (match_operand:FMSA 0 "register_operand" "=3Df")
>+	(unspec:FMSA [(match_operand:FMSA 1 "register_operand" "f")]
>+		     UNSPEC_MSA_FRCP))]
>+  "ISA_HAS_MSA"
>+  "frcp.<msafmt>\t%w0,%w1"
>+  [(set_attr "type" "simd_fdiv")
>+   (set_attr "mode" "<MODE>")])

This can be standard RTL with unsafe math opts I believe. Can be left
as a follow on.

>+(define_insn "msa_frsqrt_<msafmt>"
>+  [(set (match_operand:FMSA 0 "register_operand" "=3Df")
>+	(unspec:FMSA [(match_operand:FMSA 1 "register_operand" "f")]
>+		     UNSPEC_MSA_FRSQRT))]
>+  "ISA_HAS_MSA"
>+  "frsqrt.<msafmt>\t%w0,%w1"
>+  [(set_attr "type" "simd_fdiv")
>+   (set_attr "mode" "<MODE>")])

Likewise.

>+(define_insn "msa_ftq_h"
>+  [(set (match_operand:V8HI 0 "register_operand" "=3Df")
>+	(unspec:V8HI [(match_operand:V4SF 1 "register_operand" "f")
>+		      (match_operand:V4SF 2 "register_operand" "f")]
>+		     UNSPEC_MSA_FTQ))]
>+  "ISA_HAS_MSA"
>+  "ftq.h\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_fcvt")
>+   (set_attr "cnv_mode" "S2I")
>+   (set_attr "mode" "V4SF")])

These should be made into standard RTL as follow on work to add fixed
point modes.

>+(define_insn "msa_ftq_w"
>+  [(set (match_operand:V4SI 0 "register_operand" "=3Df")
>+	(unspec:V4SI [(match_operand:V2DF 1 "register_operand" "f")
>+		      (match_operand:V2DF 2 "register_operand" "f")]
>+		     UNSPEC_MSA_FTQ))]
>+  "ISA_HAS_MSA"
>+  "ftq.w\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_fcvt")
>+   (set_attr "cnv_mode" "D2I")
>+   (set_attr "mode" "V2DF")])

Likewise.

>+
>+(define_insn "msa_hadd_s_<msafmt>"
>+  [(set (match_operand:IMSA_DWH 0 "register_operand" "=3Df")
>+	(unspec:IMSA_DWH [(match_operand:<VHMODE> 1 "register_operand" "f")
>+			  (match_operand:<VHMODE> 2 "register_operand" "f")]
>+			 UNSPEC_MSA_HADD_S))]
>+  "ISA_HAS_MSA"
>+  "hadd_s.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])

This can be standard RTL.

>+
>+(define_insn "msa_hadd_u_<msafmt>"
>+  [(set (match_operand:IMSA_DWH 0 "register_operand" "=3Df")
>+	(unspec:IMSA_DWH [(match_operand:<VHMODE> 1 "register_operand" "f")
>+			  (match_operand:<VHMODE> 2 "register_operand" "f")]
>+			 UNSPEC_MSA_HADD_U))]
>+  "ISA_HAS_MSA"
>+  "hadd_u.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_hsub_s_<msafmt>"
>+  [(set (match_operand:IMSA_DWH 0 "register_operand" "=3Df")
>+	(unspec:IMSA_DWH [(match_operand:<VHMODE> 1 "register_operand" "f")
>+			  (match_operand:<VHMODE> 2 "register_operand" "f")]
>+			 UNSPEC_MSA_HSUB_S))]
>+  "ISA_HAS_MSA"
>+  "hsub_s.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_hsub_u_<msafmt>"
>+  [(set (match_operand:IMSA_DWH 0 "register_operand" "=3Df")
>+	(unspec:IMSA_DWH [(match_operand:<VHMODE> 1 "register_operand" "f")
>+			  (match_operand:<VHMODE> 2 "register_operand" "f")]
>+			 UNSPEC_MSA_HSUB_U))]
>+  "ISA_HAS_MSA"
>+  "hsub_u.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])

Likewise to here.

I have not reviewed interleave patterns in detail as they all look OK and
I trust they 'do the right thing'.

>+(define_insn "msa_madd_q_<msafmt>"
>+  [(set (match_operand:IMSA_WH 0 "register_operand" "=3Df")
>+	(unspec:IMSA_WH [(match_operand:IMSA_WH 1 "register_operand" "0")
>+			 (match_operand:IMSA_WH 2 "register_operand" "f")
>+			 (match_operand:IMSA_WH 3 "register_operand" "f")]
>+			UNSPEC_MSA_MADD_Q))]
>+  "ISA_HAS_MSA"
>+  "madd_q.<msafmt>\t%w0,%w2,%w3"
>+  [(set_attr "type" "simd_mul")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_maddr_q_<msafmt>"
>+  [(set (match_operand:IMSA_WH 0 "register_operand" "=3Df")
>+	(unspec:IMSA_WH [(match_operand:IMSA_WH 1 "register_operand" "0")
>+			 (match_operand:IMSA_WH 2 "register_operand" "f")
>+			 (match_operand:IMSA_WH 3 "register_operand" "f")]
>+			UNSPEC_MSA_MADDR_Q))]
>+  "ISA_HAS_MSA"
>+  "maddr_q.<msafmt>\t%w0,%w2,%w3"
>+  [(set_attr "type" "simd_mul")
>+   (set_attr "mode" "<MODE>")])

Follow on work for fixed point.

>+(define_insn "msa_max_a_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "f")
>+		      (match_operand:IMSA 2 "register_operand" "f")]
>+		     UNSPEC_MSA_MAX_A))]
>+  "ISA_HAS_MSA"
>+  "max_a.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])

This could be used as part of mov<mode>cc support.

>+(define_insn "smax<mode>3"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(smax:IMSA (match_operand:IMSA 1 "register_operand" "f")
>+		   (match_operand:IMSA 2 "register_operand" "f")))]
>+  "ISA_HAS_MSA"
>+  "max_s.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "umax<mode>3"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(umax:IMSA (match_operand:IMSA 1 "register_operand" "f")
>+		   (match_operand:IMSA 2 "register_operand" "f")))]
>+  "ISA_HAS_MSA"
>+  "max_u.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_maxi_s_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "f")
>+		      (match_operand 2 "const_imm5_operand" "")]
>+		     UNSPEC_MSA_MAXI_S))]
>+  "ISA_HAS_MSA"
>+  "maxi_s.<msafmt>\t%w0,%w1,%2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])

This should be part of the instructions above with a second alternative.

>+
>+(define_insn "msa_maxi_u_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "f")
>+		      (match_operand 2 "const_uimm5_operand" "")]
>+		     UNSPEC_MSA_MAXI_U))]
>+  "ISA_HAS_MSA"
>+  "maxi_u.<msafmt>\t%w0,%w1,%2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])

Likewise.

>+(define_insn "msa_min_a_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "f")
>+		      (match_operand:IMSA 2 "register_operand" "f")]
>+		     UNSPEC_MSA_MIN_A))]
>+  "ISA_HAS_MSA"
>+  "min_a.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])

This could be used as part of mov<mode>cc support.

>+(define_insn "smin<mode>3"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(smin:IMSA (match_operand:IMSA 1 "register_operand" "f")
>+		   (match_operand:IMSA 2 "register_operand" "f")))]
>+  "ISA_HAS_MSA"
>+  "min_s.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "umin<mode>3"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(umin:IMSA (match_operand:IMSA 1 "register_operand" "f")
>+		   (match_operand:IMSA 2 "register_operand" "f")))]
>+  "ISA_HAS_MSA"
>+  "min_u.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_mini_s_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "f")
>+		      (match_operand 2 "const_imm5_operand" "")]
>+		     UNSPEC_MSA_MINI_S))]
>+  "ISA_HAS_MSA"
>+  "mini_s.<msafmt>\t%w0,%w1,%2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])

This should be merged with the instructions above.

>+
>+(define_insn "msa_mini_u_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "f")
>+		      (match_operand 2 "const_uimm5_operand" "")]
>+		     UNSPEC_MSA_MINI_U))]
>+  "ISA_HAS_MSA"
>+  "mini_u.<msafmt>\t%w0,%w1,%2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])

Likewise.

>+
>+(define_insn "msa_msub_q_<msafmt>"
>+  [(set (match_operand:IMSA_WH 0 "register_operand" "=3Df")
>+	(unspec:IMSA_WH [(match_operand:IMSA_WH 1 "register_operand" "0")
>+			 (match_operand:IMSA_WH 2 "register_operand" "f")
>+			 (match_operand:IMSA_WH 3 "register_operand" "f")]
>+			UNSPEC_MSA_MSUB_Q))]
>+  "ISA_HAS_MSA"
>+  "msub_q.<msafmt>\t%w0,%w2,%w3"
>+  [(set_attr "type" "simd_mul")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_msubr_q_<msafmt>"
>+  [(set (match_operand:IMSA_WH 0 "register_operand" "=3Df")
>+	(unspec:IMSA_WH [(match_operand:IMSA_WH 1 "register_operand" "0")
>+			 (match_operand:IMSA_WH 2 "register_operand" "f")
>+			 (match_operand:IMSA_WH 3 "register_operand" "f")]
>+			UNSPEC_MSA_MSUBR_Q))]
>+  "ISA_HAS_MSA"
>+  "msubr_q.<msafmt>\t%w0,%w2,%w3"
>+  [(set_attr "type" "simd_mul")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_mul_q_<msafmt>"
>+  [(set (match_operand:IMSA_WH 0 "register_operand" "=3Df")
>+	(unspec:IMSA_WH [(match_operand:IMSA_WH 1 "register_operand" "f")
>+			 (match_operand:IMSA_WH 2 "register_operand" "f")]
>+			UNSPEC_MSA_MUL_Q))]
>+  "ISA_HAS_MSA"
>+  "mul_q.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_mul")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_mulr_q_<msafmt>"
>+  [(set (match_operand:IMSA_WH 0 "register_operand" "=3Df")
>+	(unspec:IMSA_WH [(match_operand:IMSA_WH 1 "register_operand" "f")
>+			 (match_operand:IMSA_WH 2 "register_operand" "f")]
>+			UNSPEC_MSA_MULR_Q))]
>+  "ISA_HAS_MSA"
>+  "mulr_q.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_mul")
>+   (set_attr "mode" "<MODE>")])

Rework when adding proper fixed point mode support.

>+(define_insn "msa_nloc_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "f")]
>+		     UNSPEC_MSA_NLOC))]
>+  "ISA_HAS_MSA"
>+  "nloc.<msafmt>\t%w0,%w1"
>+  [(set_attr "type" "simd_bit")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "clz<mode>2"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(clz:IMSA (match_operand:IMSA 1 "register_operand" "f")))]
>+  "ISA_HAS_MSA"
>+  "nlzc.<msafmt>\t%w0,%w1"
>+  [(set_attr "type" "simd_bit")
>+   (set_attr "mode" "<MODE>")])

Can you confirm that nlzc has a natural value when given an operand of zero?
I.e. 8 for B 16 for H 32 for W and 64 for D?

Also CLZ_DEFINED_VALUE_AT_ZERO looks like it needs updating to know the
bitsize of elements in a vector rather than the whole vector. I.e. I think
it will say the result is 128 for any vector mode.

>+
>+(define_insn "msa_nor_v_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(and:IMSA (not:IMSA (match_operand:IMSA 1 "register_operand" "f"))
>+		  (not:IMSA (match_operand:IMSA 2 "register_operand" "f"))))]
>+  "ISA_HAS_MSA"
>+  "nor.v\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_logic")
>+   (set_attr "mode" "TI")])
>+
>+(define_insn "msa_nori_b"
>+  [(set (match_operand:V16QI 0 "register_operand" "=3Df")
>+	(unspec:V16QI [(match_operand:V16QI 1 "register_operand" "f")
>+		       (match_operand 2 "const_uimm8_operand" "")]
>+		      UNSPEC_MSA_NORI_B))]
>+  "ISA_HAS_MSA"
>+  "nori.b\t%w0,%w1,%2"
>+  [(set_attr "type" "simd_logic")
>+   (set_attr "mode" "V16QI")])

This can be standard rtl and joined with the previous insn. The immediate
for HI/SI/DI patterns simply must have replicates byte values as well as
elements.

>+
>+(define_insn "msa_ori_b"
>+  [(set (match_operand:V16QI 0 "register_operand" "=3Df")
>+	(ior:V16QI (match_operand:V16QI 1 "register_operand" "f")
>+		   (match_operand 2 "const_uimm8_operand" "")))]
>+  "ISA_HAS_MSA"
>+  "ori.b\t%w0,%w1,%2"
>+  [(set_attr "type" "simd_logic")
>+   (set_attr "mode" "V16QI")])

This should be part of register based OR with the same rules as above.

>+(define_insn "msa_shf_<msafmt>"
>+  [(set (match_operand:IMSA_WHB 0 "register_operand" "=3Df")
>+	(unspec:IMSA_WHB [(match_operand:IMSA_WHB 1 "register_operand" "f")
>+			  (match_operand 2 "const_uimm8_operand" "")]
>+			 UNSPEC_MSA_SHF))]
>+  "ISA_HAS_MSA"
>+  "shf.<msafmt>\t%w0,%w1,%2"
>+  [(set_attr "type" "simd_shf")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_shf_w_f"
>+  [(set (match_operand:V4SF 0 "register_operand" "=3Df")
>+	(unspec:V4SF [(match_operand:V4SF 1 "register_operand" "f")
>+		      (match_operand 2 "const_uimm8_operand" "")]
>+		     UNSPEC_MSA_SHF))]
>+  "ISA_HAS_MSA"
>+  "shf.w\t%w0,%w1,%2"
>+  [(set_attr "type" "simd_shf")
>+   (set_attr "mode" "V4SF")])

These seem representable in RTL albeit ugly. Perhaps look at this in a
follow up.

>+(define_insn "msa_slli_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "f")
>+		      (match_operand 2 "const_<bitimm>_operand" "")]
>+		     UNSPEC_MSA_SLLI))]
>+  "ISA_HAS_MSA"
>+  "slli.<msafmt>\t%w0,%w1,%2"
>+  [(set_attr "type" "simd_shift")
>+   (set_attr "mode" "<MODE>")])

vashl, vashr, vlshr SPNs cover these. At least some if not all are
already defined in this file so place switch the builtins to target
them instead of these unspecs.

>+(define_insn "msa_srai_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "f")
>+		      (match_operand 2 "const_<bitimm>_operand" "")]
>+		     UNSPEC_MSA_SRAI))]
>+  "ISA_HAS_MSA"
>+  "srai.<msafmt>\t%w0,%w1,%2"
>+  [(set_attr "type" "simd_shift")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_srar_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "f")
>+		      (match_operand:IMSA 2 "register_operand" "f")]
>+		     UNSPEC_MSA_SRAR))]
>+  "ISA_HAS_MSA"
>+  "srar.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_shift")
>+   (set_attr "mode" "<MODE>")])

The shifts with rounding can stay of course.

>+
>+(define_insn "msa_srari_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "f")
>+		      (match_operand 2 "const_<bitimm>_operand" "")]
>+		     UNSPEC_MSA_SRARI))]
>+  "ISA_HAS_MSA"
>+  "srari.<msafmt>\t%w0,%w1,%2"
>+  [(set_attr "type" "simd_shift")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_srli_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "f")
>+		      (match_operand 2 "const_<bitimm>_operand" "")]
>+		     UNSPEC_MSA_SRLI))]
>+  "ISA_HAS_MSA"
>+  "srli.<msafmt>\t%w0,%w1,%2"
>+  [(set_attr "type" "simd_shift")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_srlr_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "f")
>+		      (match_operand:IMSA 2 "register_operand" "f")]
>+		     UNSPEC_MSA_SRLR))]
>+  "ISA_HAS_MSA"
>+  "srlr.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_shift")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_srlri_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "f")
>+		      (match_operand 2 "const_<bitimm>_operand" "")]
>+		     UNSPEC_MSA_SRLRI))]
>+  "ISA_HAS_MSA"
>+  "srlri.<msafmt>\t%w0,%w1,%2"
>+  [(set_attr "type" "simd_shift")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_subs_s_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "f")
>+		      (match_operand:IMSA 2 "register_operand" "f")]
>+		     UNSPEC_MSA_SUBS_S))]
>+  "ISA_HAS_MSA"
>+  "subs_s.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])

Due for update in a follow up adding fixed point mode support.

>+
>+(define_insn "msa_subs_u_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "f")
>+		      (match_operand:IMSA 2 "register_operand" "f")]
>+		     UNSPEC_MSA_SUBS_U))]
>+  "ISA_HAS_MSA"
>+  "subs_u.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_subsuu_s_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "f")
>+		      (match_operand:IMSA 2 "register_operand" "f")]
>+		     UNSPEC_MSA_SUBSUU_S))]
>+  "ISA_HAS_MSA"
>+  "subsuu_s.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_subsus_u_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "f")
>+		      (match_operand:IMSA 2 "register_operand" "f")]
>+		     UNSPEC_MSA_SUBSUS_U))]
>+  "ISA_HAS_MSA"
>+  "subsus_u.<msafmt>\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])

likewise to here.

>+
>+(define_insn "msa_subvi_<msafmt>"
>+  [(set (match_operand:IMSA 0 "register_operand" "=3Df")
>+	(unspec:IMSA [(match_operand:IMSA 1 "register_operand" "f")
>+		      (match_operand 2 "const_uimm5_operand" "")]
>+		     UNSPEC_MSA_SUBVI))]
>+  "ISA_HAS_MSA"
>+  "subvi.<msafmt>\t%w0,%w1,%2"
>+  [(set_attr "type" "simd_int_arith")
>+   (set_attr "mode" "<MODE>")])

This should be part of the simple sub<mode>3 pattern and that pattern
should be used by the builtin.

>+
>+(define_insn "msa_xori_b"
>+  [(set (match_operand:V16QI 0 "register_operand" "=3Df")
>+	(unspec:V16QI [(match_operand:V16QI 1 "register_operand" "f")
>+		       (match_operand 2 "const_uimm8_operand" "")]
>+		      UNSPEC_MSA_XORI_B))]
>+  "ISA_HAS_MSA"
>+  "xori.b\t%w0,%w1,%2"
>+  [(set_attr "type" "simd_logic")
>+   (set_attr "mode" "V16QI")])

Similar issues as ANDI and ORI.

>+(define_insn "msa_sld_<msafmt_f>"
>+  [(set (match_operand:MSA 0 "register_operand" "=3Df")
>+	(unspec:MSA [(match_operand:MSA 1 "register_operand" "0")
>+		     (match_operand:MSA 2 "register_operand" "f")
>+		     (match_operand:SI 3 "reg_or_0_operand" "dJ")]
>+		    UNSPEC_MSA_SLD))]
>+  "ISA_HAS_MSA"
>+  "sld.<msafmt>\t%w0,%w2[%z3]"
>+  [(set_attr "type" "simd_sld")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_sldi_<msafmt_f>"
>+  [(set (match_operand:MSA 0 "register_operand" "=3Df")
>+	(unspec:MSA [(match_operand:MSA 1 "register_operand" "0")
>+		     (match_operand:MSA 2 "register_operand" "f")
>+		     (match_operand 3 "const_<indeximm>_operand" "")]
>+		    UNSPEC_MSA_SLDI))]
>+  "ISA_HAS_MSA"
>+  "sldi.<msafmt>\t%w0,%w2[%3]"
>+  [(set_attr "type" "simd_sld")
>+   (set_attr "mode" "<MODE>")])
>+
>+(define_insn "msa_splat_<msafmt_f>"
>+  [(set (match_operand:MSA 0 "register_operand" "=3Df")
>+	(unspec:MSA [(match_operand:MSA 1 "register_operand" "f")
>+		     (match_operand:SI 2 "reg_or_0_operand" "dJ")]
>+		    UNSPEC_MSA_SPLAT))]
>+  "ISA_HAS_MSA"
>+  "splat.<msafmt>\t%w0,%w1[%z2]"
>+  [(set_attr "type" "simd_splat")
>+   (set_attr "mode" "<MODE>")])

This doesn't need 'or_0' support as the splati below covers it. I think
this is targetable but seem to recall an off-list discussion that says
it can never be generated even if represented with an appropriate
pattern.

>+
>+(define_insn "msa_splati_<msafmt_f>"
>+  [(set (match_operand:MSA 0 "register_operand" "=3Df")
>+	(unspec:MSA [(match_operand:MSA 1 "register_operand" "f")
>+		     (match_operand 2 "const_<indeximm>_operand" "")]
>+		    UNSPEC_MSA_SPLATI))]
>+  "ISA_HAS_MSA"
>+  "splati.<msafmt>\t%w0,%w1[%2]"
>+  [(set_attr "type" "simd_splat")
>+   (set_attr "mode" "<MODE>")])

This seems targettable with standard RTL.

>+
>+;; Operand 1 is a scalar.
>+(define_insn "msa_splati_<msafmt_f>_s"
>+  [(set (match_operand:FMSA 0 "register_operand" "=3Df")
>+	(unspec:FMSA [(match_operand:<UNITMODE> 1 "register_operand" "f")
>+		      (match_operand 2 "const_<indeximm>_operand" "")]
>+		     UNSPEC_MSA_SPLATI))]
>+  "ISA_HAS_MSA"
>+  "splati.<msafmt>\t%w0,%w1[%2]"
>+  [(set_attr "type" "simd_splat")
>+   (set_attr "mode" "<MODE>")])

I don't understand this pattern. Why is there an element selector for
a scalar input. Isn't it just element 0 hard-coded?

>+(define_insn "msa_cfcmsa"
>+  [(set (match_operand:SI 0 "register_operand" "=3Dd")
>+	(unspec_volatile:SI [(match_operand 1 "const_uimm5_operand" "")]
>+			    UNSPEC_MSA_CFCMSA))]
>+  "ISA_HAS_MSA"
>+  "cfcmsa\t%0,$%1"
>+  [(set_attr "type" "simd_cmsa")
>+   (set_attr "mode" "SI")])
>+
>+(define_insn "msa_ctcmsa"
>+  [(unspec_volatile [(match_operand 0 "const_uimm5_operand" "")
>+		     (match_operand:SI 1 "register_operand" "d")]
>+		    UNSPEC_MSA_CTCMSA)]
>+  "ISA_HAS_MSA"
>+  "ctcmsa\t$%0,%1"
>+  [(set_attr "type" "simd_cmsa")
>+   (set_attr "mode" "SI")])

Just noting that the CTCMSA instruction is defined with arguments backwards
to other ctc* instructions in the base arch. The copro register always goes
second apart from CTCMSA.

>+
>+(define_insn "msa_fexdo_h"
>+  [(set (match_operand:V8HI 0 "register_operand" "=3Df")
>+	(unspec:V8HI [(match_operand:V4SF 1 "register_operand" "f")
>+		      (match_operand:V4SF 2 "register_operand" "f")]
>+		     UNSPEC_MSA_FEXDO))]
>+  "ISA_HAS_MSA"
>+  "fexdo.h\t%w0,%w1,%w2"
>+  [(set_attr "type" "simd_fcvt")
>+   (set_attr "mode" "V8HI")])
>+
>+(define_insn "msa_fexdo_w"
>+  [(set (match_operand:V4SF 0 "register_operand" "=3Df")
>+	(vec_concat:V4SF
>+	  (float_truncate:V2SF (match_operand:V2DF 1 "register_operand" "f"))
>+	  (float_truncate:V2SF (match_operand:V2DF 2 "register_operand" "f"))))]
>+  "ISA_HAS_MSA"
>+  "fexdo.w\t%w0,%w2,%w1"
>+  [(set_attr "type" "simd_fcvt")
>+   (set_attr "mode" "V4SF")])
>+
>+(define_insn "msa_fexupl_w"
>+  [(set (match_operand:V4SF 0 "register_operand" "=3Df")
>+	(unspec:V4SF [(match_operand:V8HI 1 "register_operand" "f")]
>+		     UNSPEC_MSA_FEXUPL))]
>+  "ISA_HAS_MSA"
>+  "fexupl.w\t%w0,%w1"
>+  [(set_attr "type" "simd_fcvt")
>+   (set_attr "mode" "V4SF")])
>+
>+(define_insn "msa_fexupl_d"
>+  [(set (match_operand:V2DF 0 "register_operand" "=3Df")
>+	(unspec:V2DF [(match_operand:V4SF 1 "register_operand" "f")]
>+		     UNSPEC_MSA_FEXUPL))]
>+  "ISA_HAS_MSA"
>+  "fexupl.d\t%w0,%w1"
>+  [(set_attr "type" "simd_fcvt")
>+   (set_attr "mode" "V2DF")])

This should be possible with float_extend similar to fexdo_w

>+
>+(define_insn "msa_fexupr_w"
>+  [(set (match_operand:V4SF 0 "register_operand" "=3Df")
>+       (unspec:V4SF [(match_operand:V8HI 1 "register_operand" "f")]
>+		    UNSPEC_MSA_FEXUPR))]
>+  "ISA_HAS_MSA"
>+  "fexupr.w\t%w0,%w1"
>+  [(set_attr "type" "simd_fcvt")
>+   (set_attr "mode" "V4SF")])

Likewise.

>+(define_insn "msa_fexupr_d"
>+  [(set (match_operand:V2DF 0 "register_operand" "=3Df")
>+       (unspec:V2DF [(match_operand:V4SF 1 "register_operand" "f")]
>+		    UNSPEC_MSA_FEXUPR))]
>+  "ISA_HAS_MSA"
>+  "fexupr.d\t%w0,%w1"
>+  [(set_attr "type" "simd_fcvt")
>+   (set_attr "mode" "V2DF")])
>+
>+(define_insn "msa_branch_nz_v_<msafmt_f>"
>+ [(set (pc) (if_then_else
>+	      (ne (unspec:SI [(match_operand:MSA 1 "register_operand" "f")]
>+			     UNSPEC_MSA_BNZ_V)
>+		  (match_operand:SI 2 "const_0_operand"))
>+		  (label_ref (match_operand 0))
>+		  (pc)))]
>+ "ISA_HAS_MSA"
>+{
>+  return mips_output_conditional_branch (insn, operands,
>+					 MIPS_BRANCH ("bnz.v", "%w1,%0"),
>+					 MIPS_BRANCH ("bz.v", "%w1,%0"));
>+}
>+ [(set_attr "type" "simd_branch")
>+  (set_attr "mode" "TI")])

This needs updating for compact branch logic. See the floating point branch=
es
for reference, it must be attribute compact_form=3D=3Dnever.
This can be a proper RTL pattern quite easily with a comparison against
a const_vector with all zeros. The eq and ne cases should be merged
together with a template (see other branches).

>+
>+(define_expand "msa_bnz_v_<msafmt_f>"
>+  [(set (match_operand:SI 0 "register_operand" "=3Dd")
>+	(unspec:SI [(match_operand:MSA 1 "register_operand" "f")]
>+		   UNSPEC_MSA_TSTNZ_V))]
>+  "ISA_HAS_MSA"
>+{
>+  mips_expand_msa_branch (operands, gen_msa_branch_nz_v_<MSA:msafmt_f>);
>+  DONE;
>+})

I find these instruction definitions very weird. Why is it a good thing to
expose branches as intrinsics that end up with a GPR value that has to then
be used in another branch to create control flow?
Iff there is some value to these then the function name for expanding them
is a bit misleading as it is not really branching anywhere it is just setti=
ng
a GPR to 1 or 0 which happens to include a set of if-then-else branchs. It
would be nice to avoid having a define_expand at all here and use a custom
builtin type to expand it entirely in C.

>+(define_insn "msa_branchz_v_<msafmt_f>"
>+ [(set (pc) (if_then_else
>+	      (eq (unspec:SI [(match_operand:MSA 1 "register_operand" "f")]
>+			     UNSPEC_MSA_BZ_V)
>+		  (match_operand:SI 2 "const_0_operand"))
>+		  (label_ref (match_operand 0))
>+		  (pc)))]
>+ "ISA_HAS_MSA"
>+{
>+  return mips_output_conditional_branch (insn, operands,
>+					 MIPS_BRANCH ("bz.v", "%w1,%0"),
>+					 MIPS_BRANCH ("bnz.v", "%w1,%0"));
>+}
>+ [(set_attr "type" "simd_branch")
>+  (set_attr "mode" "TI")])

Merge with msa_branch_nz_v_<msafmt_f>.

>+(define_expand "msa_bz_v_<msafmt_f>"
>+  [(set (match_operand:SI 0 "register_operand" "=3Dd")
>+	(unspec:SI [(match_operand:MSA 1 "register_operand" "f")]
>+		   UNSPEC_MSA_TSTZ_V))]
>+  "ISA_HAS_MSA"
>+{
>+  mips_expand_msa_branch (operands, gen_msa_branchz_v_<MSA:msafmt_f>);
>+  DONE;
>+})

Similar to msa_bnz_v_<msafmt_f>;

>+(define_insn "msa_branchnz_<msafmt_f>"
>+ [(set (pc) (if_then_else
>+	      (ne (unspec:SI [(match_operand:MSA 1 "register_operand" "f")]
>+			     UNSPEC_MSA_BNZ)
>+		  (match_operand:SI 2 "const_0_operand"))
>+		  (label_ref (match_operand 0))
>+		  (pc)))]
>+ "ISA_HAS_MSA"
>+{
>+  return mips_output_conditional_branch (insn, operands,
>+					 MIPS_BRANCH ("bnz.<msafmt>", "%w1,%0"),
>+					 MIPS_BRANCH ("bz.<msafmt>", "%w1,%0"));
>+
>+}
>+

whitespace.

>+ [(set_attr "type" "simd_branch")
>+  (set_attr "mode" "<MODE>")])

As above I find the branch instructions a bit pointless unless we can use t=
hem
as actual branches. This branch could be represented as an 'IOR' of all ele=
ments
and a test against zero. I don't know if that can be generated. These need
some more thought but it can be done as a follow on. These patterns do need
compact_form=3Dnever applying.

>+
>+(define_expand "msa_bnz_<msafmt>"
>+  [(set (match_operand:SI 0 "register_operand" "=3Dd")
>+	(unspec:SI [(match_operand:IMSA 1 "register_operand" "f")]
>+		   UNSPEC_MSA_TSTNZ))]
>+  "ISA_HAS_MSA"
>+{
>+  mips_expand_msa_branch (operands, gen_msa_branchnz_<IMSA:msafmt>);
>+  DONE;
>+})
>+
>+(define_insn "msa_branchz_<msafmt>"
>+ [(set (pc) (if_then_else
>+	      (eq (unspec:SI [(match_operand:IMSA 1 "register_operand" "f")]
>+			     UNSPEC_MSA_BZ)
>+		   (match_operand:IMSA 2 "const_0_operand"))
>+		  (label_ref (match_operand 0))
>+		  (pc)))]
>+ "ISA_HAS_MSA"
>+{
>+  return mips_output_conditional_branch (insn, operands,
>+					 MIPS_BRANCH ("bz.<msafmt>", "%w1,%0"),
>+					 MIPS_BRANCH ("bnz.<msafmt>","%w1,%0"));
>+}
>+ [(set_attr "type" "simd_branch")
>+  (set_attr "mode" "<MODE>")])
>+
>+(define_expand "msa_bz_<msafmt>"
>+  [(set (match_operand:SI 0 "register_operand" "=3Dd")
>+	(unspec:SI [(match_operand:IMSA 1 "register_operand" "f")]
>+		   UNSPEC_MSA_TSTZ))]
>+  "ISA_HAS_MSA"
>+{
>+  mips_expand_msa_branch (operands, gen_msa_branchz_<IMSA:msafmt>);
>+  DONE;
>+})
>+
>+;; Note that this instruction treats scalar as vector registers freely.
>+(define_insn "msa_cast_to_vector_<msafmt_f>"
>+  [(set (match_operand:FMSA 0 "register_operand" "=3Df")
>+	(unspec:FMSA [(match_operand:<UNITMODE> 1 "register_operand" "f")]
>+		     UNSPEC_MSA_CAST_TO_VECTOR))]
>+  "ISA_HAS_MSA"
>+{
>+  if (REGNO (operands[0]) =3D=3D REGNO (operands[1]))
>+    return "nop\t# Cast %1 to %w0";
>+  else
>+    return "mov.<unitfmt>\t%0,%1\t# Cast %1 to %w0";
>+}
>+  [(set_attr "type" "arith")
>+   (set_attr "mode" "TI")])

This is simply INSVE but implemented in a weird way. Unless you can explain
their value please delete them :-)

>+
>+;; Note that this instruction treats vector as scalar registers freely.
>+(define_insn "msa_cast_to_scalar_<msafmt_f>"
>+  [(set (match_operand:<UNITMODE> 0 "register_operand" "=3Df")
>+	(unspec:<UNITMODE> [(match_operand:FMSA 1 "register_operand" "f")]
>+			   UNSPEC_MSA_CAST_TO_SCALAR))]
>+  "ISA_HAS_MSA"
>+{
>+  if (REGNO (operands[0]) =3D=3D REGNO (operands[1]))
>+    return "nop\t# Cast %w1 to %0";
>+  else
>+    return "mov.<unitfmt>\t%0,%1\t# Cast %w1 to %0";
>+}
>+  [(set_attr "type" "arith")
>+   (set_attr "mode" "TI")])

Likewise. Update vec_extract to use a simple move with a subreg instead
and leave the other patterns to sort it out. There may be a bit of complexi=
ty
to deal with here but this certainly looks like the wrong way to solve the
problem to me.

Thanks,
Matthew