On 12/13/19 11:02 AM, Richard Sandiford wrote:
> Stam Markianos-Wright <Stam.Markianos-Wright@arm.com> writes:
>> @@ -573,6 +586,44 @@
>>     [(set_attr "type" "neon_dot<q>")]
>>   )
>>   
>> +;; These instructions map to the __builtins for the armv8.6a I8MM usdot, sudot
>> +;; (by element) Dot Product operations.
>> +(define_insn "aarch64_<sur>dot_lane<vsi2qi>"
>> +  [(set (match_operand:VS 0 "register_operand" "=w")
>> +	(plus:VS (match_operand:VS 1 "register_operand" "0")
>> +		(unspec:VS [(match_operand:<VSI2QI> 2 "register_operand" "w")
>> +			    (match_operand:V8QI 3 "register_operand" "<h_con>")
>> +			    (match_operand:SI 4 "immediate_operand" "i")]
>> +		DOTPROD_I8MM)))]
>> +  "TARGET_SIMD && TARGET_I8MM"
>> +  {
>> +    int nunits = GET_MODE_NUNITS (V8QImode).to_constant ();
>> +    int lane = INTVAL (operands[4]);
>> +    operands[4]
>> +    =  gen_int_mode (ENDIAN_LANE_N (nunits / 4, lane), SImode);
>> +    return "<sur>dot\\t%0.<Vtype>, %2.<Vdottype>, %3.4b[%4]";
>> +  }
>> +  [(set_attr "type" "neon_dot<q>")]
>> +)
>> +
>> +(define_insn "aarch64_<sur>dot_laneq<vsi2qi>"
>> +  [(set (match_operand:VS 0 "register_operand" "=w")
>> +	(plus:VS (match_operand:VS 1 "register_operand" "0")
>> +		(unspec:VS [(match_operand:<VSI2QI> 2 "register_operand" "w")
>> +			    (match_operand:V16QI 3 "register_operand" "<h_con>")
> 
> Using <h_con> seems a bit redundant when it's always "w" in this context,
> but either's fine.

Done!

> 
>> +			    (match_operand:SI 4 "immediate_operand" "i")]
>> +		DOTPROD_I8MM)))]
>> +  "TARGET_SIMD && TARGET_I8MM"
>> +  {
>> +    int nunits = GET_MODE_NUNITS (V16QImode).to_constant ();
>> +    int lane = INTVAL (operands[4]);
>> +    operands[4]
>> +    =  gen_int_mode (ENDIAN_LANE_N (nunits / 4, lane), SImode);
> 
> Nit: = should be indented two spaces more, and there should be only
> one space afterwards.  But the statement fits on one line, so probably
> better not to have the line break at all.

I put put all onto one line.
> 
>> +    return "<sur>dot\\t%0.<Vtype>, %2.<Vdottype>, %3.4b[%4]";
>> +  }
>> +  [(set_attr "type" "neon_dot<q>")]
>> +)
> 
> These two patterns can be merged using :VB for operand 3.

Merged them.

I also changed the tests to use the new check-function-bodies according to 
downstream comments.
This helps check that the assembler scans are done in the right order and 
ensures that the correct assembler was generated from the right function call 
(as opposed to "somewhere in the output file").

Hope this looks better :D

Cheers,
Stam
> 
> LGTM otherwise, thanks.
> 
> Richard
>