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

[Matthew Fortune]

Hi Robert,

Apologies for the long delay again. This patch is hard to get through. My comments
are not all in source sequence but I've tried to keep them short. With a few minor
things fixed and some trivial style issues done then this is ready to go. I've left
a number of things to look at after getting this patch in as I can't track any more
significant updates to this:

> mips_gen_const_int_vector
This should use gen_int_for_mode instead of GEN_INT to avoid the issues that msa_ldi is
trying to handle.

> mips_const_vector_same_bytes_p
comment on this function is same as previous function

> mips_msa_idiv_insns
Why not just update mips_idiv_insns and add a mode argument?

> Implement TARGET_PRINT_OPERAND.
Comment spacing between 'E' 'B' and description is different to existing

> mips_print_operand
case 'v' subcases V4SImode and V4SFmode are identical. same for DI/DF.

>@@ -12272,13 +12837,25 @@ mips_class_max_nregs (enum reg_class rclass, machine_mode mode)
>   if (hard_reg_set_intersect_p (left, reg_class_contents[(int) ST_REGS]))
>     {
>       if (HARD_REGNO_MODE_OK (ST_REG_FIRST, mode))
>-	size = MIN (size, 4);
>+	{
>+	  if (MSA_SUPPORTED_MODE_P (mode))
>+	    size = MIN (size, UNITS_PER_MSA_REG);
>+	  else
>+	    size = MIN (size, UNITS_PER_FPREG);
>+	}
>+

This hunk should be removed. MSA modes are not supported in ST_REGS.

>@@ -12299,6 +12876,10 @@ mips_cannot_change_mode_class (machine_mode from,
>       && INTEGRAL_MODE_P (from) && INTEGRAL_MODE_P (to))
>     return false;
> 
>+  /* Allow conversions between different MSA vector modes and TImode.  */

Remove 'and TImode' we do not support it.

>@@ -19497,9 +21284,64 @@ mips_expand_vec_unpack (rtx operands[2], bool unsigned_p, bool high_p)
>+    if (!unsigned_p)
>+    {
>+      /* Extract sign extention for each element comparing each element with
>+	 immediate zero.  */
>+      tmp = gen_reg_rtx (imode);
>+      emit_insn (cmpFunc (tmp, operands[1], CONST0_RTX (imode)));
>+    }
>+    else
>+    {
>+      tmp = force_reg (imode, CONST0_RTX (imode));
>+    }

Indentation and unnecessary braces on the else.

+   A single N-word move is usually the same cost as N single-word moves.
+   For MSA, we set MOVE_MAX to 16 bytes.
+   Then, MAX_MOVE_MAX is 16 unconditionally.  */
+#define MOVE_MAX (TARGET_MSA ? 16 : UNITS_PER_WORD)
+#define MAX_MOVE_MAX 16

The 16 here should be UNITS_PER_MSA_REG

> mips_expand_builtin_insn

General comment about operations that take an immediate. There is code to perform range
checking but it does not seem to leave any trail when the maybe_expand_insn fails to 
tell the user it was an out of range immediate that was the problem. (follow up work)

>+    case CODE_FOR_msa_andi_b:
>+    case CODE_FOR_msa_ori_b:
>+    case CODE_FOR_msa_nori_b:
>+    case CODE_FOR_msa_xori_b:
>+      gcc_assert (has_target_p && nops == 3);
>+      if (!CONST_INT_P (ops[2].value))
>+	break;
>+      ops[2].mode = ops[0].mode;
>+      /* We need to convert the unsigned value to signed.  */
>+      val = sext_hwi (INTVAL (ops[2].value),
>+		      GET_MODE_UNIT_PRECISION (ops[2].mode));
>+      ops[2].value = mips_gen_const_int_vector (ops[2].mode, val);
>+      break

Isn't the sext_hwi just effectively doing what gen_int_for_mode would? Fixing
mips_gen_const_int_vector would eliminate all of them.

>@@ -527,7 +551,9 @@ (define_attr "insn_count" ""
> 	 (const_int 2)
> 
> 	 (eq_attr "type" "idiv,idiv3")
>-	 (symbol_ref "mips_idiv_insns ()")
>+	 (cond [(eq_attr "mode" "TI")
>+		(symbol_ref "mips_msa_idiv_insns () * 4")]
>+		(symbol_ref "mips_idiv_insns () * 4"))

Why *4?

>@@ -1537,8 +1553,10 @@ FP_ASM_SPEC "\
> #define LONG_LONG_ACCUM_TYPE_SIZE (TARGET_64BIT ? 128 : 64)
> 
> /* long double is not a fixed mode, but the idea is that, if we
>-   support long double, we also want a 128-bit integer type.  */
>-#define MAX_FIXED_MODE_SIZE LONG_DOUBLE_TYPE_SIZE
>+   support long double, we also want a 128-bit integer type.
>+   For MSA, we support an integer type with a width of BITS_PER_MSA_REG.  */
>+#define MAX_FIXED_MODE_SIZE \
>+  (TARGET_MSA ? BITS_PER_MSA_REG : LONG_DOUBLE_TYPE_SIZE)

This doesn't seem right. We don't support TImode with MSA.

>diff --git a/gcc/config/mips/mips-msa.md b/gcc/config/mips/mips-msa.md
>new file mode 100644
>index 0000000..79e382d
>--- /dev/null
>+++ b/gcc/config/mips/mips-msa.md
>@@ -0,0 +1,2725 @@
>+(define_insn "msa_copy_s_<msafmt_f>"
>+  [(set (match_operand:<UNITMODE> 0 "register_operand" "=d")
>+	(vec_select:<UNITMODE>
>+	  (match_operand:MSA_W 1 "register_operand" "f")
>+	  (parallel [(match_operand 2 "const_<indeximm>_operand" "")])))]
>+  "ISA_HAS_MSA"
>+  "copy_s.<msafmt>\t%0,%w1[%2]"
>+  [(set_attr "type" "simd_copy")
>+   (set_attr "mode" "<MODE>")])

There is a sign extend version of this pattern needed for TARGET_64BIT widening
to DImode.

>+(define_expand "msa_ldi<mode>"
>+  [(match_operand:IMSA 0 "register_operand")
>+   (match_operand 1 "const_imm10_operand")]
>+  "ISA_HAS_MSA"
>+{
>+  if (<MODE>mode == V16QImode)
>+    operands[1] = GEN_INT (trunc_int_for_mode (INTVAL (operands[1]),
>+					       <UNITMODE>mode));

I still don't like this expander. I think it needs moving to the builtin
expansion code as a follow up.

> "msa_fill_<msafmt_f>"
The fill with constant could be extended to handle all immediates for LDI including
those for which the constant is wider that 8-bit but contains a replicated value that
a narrower LDI could create. (Just for follow up work)

General comment: A number of TARGET_MSA instances should be ISA_HAS_MSA please check.

I'm not sure but I don't think the mapping from FP comparisons to signalling vs quiet
compares is correct. It needs checking in detail for a follow up.

Thanks,
Matthew