This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: [PATCH 1/4] [MIPS] Add support for MIPS SIMD Architecture (MSA)


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 = 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 = 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] = 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 = GET_MODE_NUNITS (<MODE>mode);
>+  rtvec v = rtvec_alloc (n_elts);
>+  HOST_WIDE_INT val = INTVAL (operands[1]);
>+  unsigned int i;
>+
>+  if (<MODE>mode != V16QImode)
>+    {
>+      unsigned shift = HOST_BITS_PER_WIDE_INT - 10;
>+      val = trunc_int_for_mode ((val << shift) >> shift, <UNITMODE>mode);
>+    }
>+  else
>+    val = trunc_int_for_mode (val, <UNITMODE>mode);
>+
>+  for (i = 0; i < n_elts; i++)
>+    RTVEC_ELT (v, i) = 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" "=f")
>+	(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 allowing

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 = 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 = 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. 
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" "=f,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 = INTVAL (CONST_VECTOR_ELT (operands[2], 0));
>+
>+	operands[2] = GEN_INT (-val);
>+	return "subvi.<msafmt>\t%w0,%w1,%d2";
>+      }
>+    case 2:
>+      {
>+	HOST_WIDE_INT val = INTVAL (CONST_VECTOR_ELT (operands[2], 0));
>+
>+	operands[2] = 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" "=f,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 = INTVAL (CONST_VECTOR_ELT (operands[2], 0));
>+
>+	operands[2] = GEN_INT (-val);
>+	return "addvi.<msafmt>\t%w0,%w1,%d2";
>+      }
>+    case 2:
>+      {
>+	HOST_WIDE_INT val = INTVAL (CONST_VECTOR_ELT (operands[2], 0));
>+
>+	operands[2] = 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" "=f,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 == 1)
>+    {
>+      operands[2] = 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"
				
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" "=f,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,YC")))]
>+  "ISA_HAS_MSA"
>+{
>+  if (which_alternative == 1)
>+    {
>+      HOST_WIDE_INT val = INTVAL (CONST_VECTOR_ELT (operands[2], 0));
>+      int vlog2 = exact_log2 (val);
>+      gcc_assert (vlog2 != -1);
>+      operands[2] = GEN_INT (vlog2);
>+      return "bnegi.%v0\t%w0,%w1,%2";

If this pattern occurs frequently then this should switch to a formatter and 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" "=f,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 == 1)
>+    {
>+      operands[2] = 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" "=f,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 == 1)
>+    {
>+      operands[2] = 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" "=f,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 == 0)
>+    return "srl.<msafmt>\t%w0,%w1,%w2";
>+
>+  operands[2] = 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" "=f,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 == 0)
>+    return "sra.<msafmt>\t%w0,%w1,%w2";
>+
>+  operands[2] = 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" "=f,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 == 0)
>+    return "sll.<msafmt>\t%w0,%w1,%w2";
>+
>+  operands[2] = 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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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 = GET_MODE_NUNITS (<MODE>mode);
>+  rtvec v = rtvec_alloc (n_elts);
>+  HOST_WIDE_INT val = INTVAL (operands[2]);
>+  unsigned int i;
>+
>+  for (i = 0; i < n_elts; i++)
>+    RTVEC_ELT (v, i) = 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. 

>+
>+(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 = rtvec_alloc (16);
>+  HOST_WIDE_INT val = INTVAL (operands[2]);
>+  unsigned int i;
>+
>+  for (i = 0; i < 16; i++)
>+    RTVEC_ELT (v, i) = 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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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 questionable.

>+(define_insn "msa_dotp_u_<msafmt>"
>+  [(set (match_operand:IMSA_DWH 0 "register_operand" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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 support
in follow on work.

>+(define_insn "msa_ffqr_<msafmt>"
>+  [(set (match_operand:FMSA 0 "register_operand" "=f")
>+	(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" "=f")
>+	(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]) == SUBREG
>+				  && REG_P (SUBREG_REG (operands[1])))))
>+    operands[1] = lowpart_subreg (<UNITMODE>mode, operands[1], <RES>mode);
>+  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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=d")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+	(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" "=f")
>+       (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" "=f")
>+       (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 branches
for reference, it must be attribute compact_form==never.
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" "=d")
>+	(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 setting
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" "=d")
>+	(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 them
as actual branches. This branch could be represented as an 'IOR' of all elements
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=never applying.

>+
>+(define_expand "msa_bnz_<msafmt>"
>+  [(set (match_operand:SI 0 "register_operand" "=d")
>+	(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" "=d")
>+	(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" "=f")
>+	(unspec:FMSA [(match_operand:<UNITMODE> 1 "register_operand" "f")]
>+		     UNSPEC_MSA_CAST_TO_VECTOR))]
>+  "ISA_HAS_MSA"
>+{
>+  if (REGNO (operands[0]) == 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" "=f")
>+	(unspec:<UNITMODE> [(match_operand:FMSA 1 "register_operand" "f")]
>+			   UNSPEC_MSA_CAST_TO_SCALAR))]
>+  "ISA_HAS_MSA"
>+{
>+  if (REGNO (operands[0]) == 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 complexity
to deal with here but this certainly looks like the wrong way to solve the
problem to me.

Thanks,
Matthew


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]