On 6/19/20 11:45 AM, Segher Boessenkool wrote:
> 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"?
Yes, that's a better name. Changed.
>> + 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?
Ok. I'm actually surprised this wasn't added as part of the
initial -mcpu=future patch that went in a while ago.
>> @@ -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.
Ok, I'll make those changes.
>> +(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?
Must be a cut/paste error and probably why we saw mode == PXImode
in the mma_input_operand predicate. I'll change that and the
predicate and retest. Thanks for pointing that out!
> Anyway, okay for trunk. Thanks! Thanks to all who worked on this, it
> was a painful trip getting to here.
Thanks for the review!
Peter
