Hi!
On Thu, Jun 18, 2020 at 03:45:17PM -0500, Peter Bergner wrote:
> +;; Return 1 if this operand is valid for a MMA assemble accumulator insn.
> +(define_special_predicate "mma_input_operand"
> + (match_test "(mode == PXImode
> + && (GET_MODE (op) == V16QImode)
> + && (vsx_register_operand (op, GET_MODE (op)) || MEM_P (op)))"))
Maybe the name could be better, then? "mma_assemble_input_operand"?
I don't see how mode is PXI but GET_MODE (op) is V16QI. The actual
register is V16QI, but then it is used as if it was a PXI? Is there no
better way to do this? It certainly needs a comment, and a more specific
name, if you keep this like this.
> +BU_MMA_6 (PMXVI16GER2SPP, "pmxvi16ger2spp", QUAD, mma_pmxvi16ger2spp)
(I didn't check the large table, I'll just hope you did -- check against
the ISA and the builtins docs, etc. :-) )
> + else if ((fnmask & RS6000_BTM_FUTURE) != 0)
> + error ("%qs requires the %qs option", name, "-mcpu=future");
In the future, please send such pieces in a separate patch?
> @@ -9944,7 +9944,8 @@ rs6000_emit_move (rtx dest, rtx source, machine_mode
> mode)
>
> case E_POImode:
> case E_PXImode:
> - if (CONSTANT_P (operands[1]))
> + if (CONSTANT_P (operands[1])
> + && INTVAL (operands[1]) != 0)
> error ("%qs is an opaque type, and you can't set it to other values.",
> (mode == POImode) ? "__vector_pair" : "__vector_quad");
Put that condition on just one line please? CONSTANT_P might not be
good enough if you want do use INTVAL btw, CONST_INT_P is clearer and/or
more correct.
> +(define_insn_and_split "*mma_assemble_acc"
> + [(set (match_operand:PXI 0 "fpr_reg_operand" "=d")
> + (unspec:PXI [(match_operand:PXI 1 "mma_input_operand" "mwa")
> + (match_operand:PXI 2 "mma_input_operand" "mwa")
> + (match_operand:PXI 3 "mma_input_operand" "mwa")
> + (match_operand:PXI 4 "mma_input_operand" "mwa")]
> + UNSPEC_MMA_ASSEMBLE_ACC))]
I would expect all those four last match_operand to be :V16QI, so why
does it use this strange mode?
In general, many of the MMA insns play loose and fast with the modes.
This probably works fine, since everything is unspec, but eww :-)
Anyway, okay for trunk. Thanks! Thanks to all who worked on this, it
was a painful trip getting to here.
Segher
