On Fri, Nov 11, 2016 at 7:23 PM, Andrew Senkevich
<andrew.n.senkev...@gmail.com> wrote:
> 2016-11-11 20:56 GMT+03:00 Uros Bizjak <ubiz...@gmail.com>:
>> On Fri, Nov 11, 2016 at 6:50 PM, Uros Bizjak <ubiz...@gmail.com> wrote:
>>> On Fri, Nov 11, 2016 at 6:38 PM, Andrew Senkevich
>>> <andrew.n.senkev...@gmail.com> wrote:
>>>> 2016-11-11 17:34 GMT+03:00 Uros Bizjak <ubiz...@gmail.com>:
>>>>> Some quick remarks:
>>>>>
>>>>> +(define_insn "kmovb"
>>>>> + [(set (match_operand:QI 0 "nonimmediate_operand" "=k,k")
>>>>> + (unspec:QI
>>>>> + [(match_operand:QI 1 "nonimmediate_operand" "r,km")]
>>>>> + UNSPEC_KMOV))]
>>>>> + "!(MEM_P (operands[0]) && MEM_P (operands[1])) && TARGET_AVX512DQ"
>>>>> + "@
>>>>> + kmovb\t{%k1, %0|%0, %k1}
>>>>> + kmovb\t{%1, %0|%0, %1}";
>>>>> + [(set_attr "mode" "QI")
>>>>> + (set_attr "type" "mskmov")
>>>>> + (set_attr "prefix" "vex")])
>>>>> +
>>>>> +(define_insn "kmovd"
>>>>> + [(set (match_operand:SI 0 "nonimmediate_operand" "=k,k")
>>>>> + (unspec:SI
>>>>> + [(match_operand:SI 1 "nonimmediate_operand" "r,km")]
>>>>> + UNSPEC_KMOV))]
>>>>> + "!(MEM_P (operands[0]) && MEM_P (operands[1])) && TARGET_AVX512BW"
>>>>> + "@
>>>>> + kmovd\t{%k1, %0|%0, %k1}
>>>>> + kmovd\t{%1, %0|%0, %1}";
>>>>> + [(set_attr "mode" "SI")
>>>>> + (set_attr "type" "mskmov")
>>>>> + (set_attr "prefix" "vex")])
>>>>> +
>>>>> +(define_insn "kmovq"
>>>>> + [(set (match_operand:DI 0 "nonimmediate_operand" "=k,k,km")
>>>>> + (unspec:DI
>>>>> + [(match_operand:DI 1 "nonimmediate_operand" "r,km,k")]
>>>>> + UNSPEC_KMOV))]
>>>>> + "!(MEM_P (operands[0]) && MEM_P (operands[1])) && TARGET_AVX512BW"
>>>>> + "@
>>>>> + kmovq\t{%k1, %0|%0, %k1}
>>>>> + kmovq\t{%1, %0|%0, %1}
>>>>> + kmovq\t{%1, %0|%0, %1}";
>>>>> + [(set_attr "mode" "DI")
>>>>> + (set_attr "type" "mskmov")
>>>>> + (set_attr "prefix" "vex")])
>>>>>
>>>>> - kmovd (and existing kmovw) should be using register_operand for
>>>>> opreand 0. In this case, there is no need for MEM_P checks at all.
>>>>> - In the insn constraint, pease check TARGET_AVX before checking MEM_P.
>>>>> - please put these definitions above corresponding *mov??_internal
>>>>> patterns.
>>>>
>>>> Do you mean put below *mov??_internal patterns? Attached corrected such
>>>> way.
>>>
>>> No, please put kmovq near *movdi_internal, kmovd near *movsi_internal,
>>> etc. It doesn't matter if they are above or below their respective
>>> *mov??_internal patterns, as long as they are positioned in some
>>> consistent way. IOW, new patterns shouldn't be grouped together, as is
>>> the case with your patch.
>>
>> +(define_insn "kmovb"
>> + [(set (match_operand:QI 0 "register_operand" "=k,k")
>> + (unspec:QI
>> + [(match_operand:QI 1 "nonimmediate_operand" "r,km")]
>> + UNSPEC_KMOV))]
>> + "TARGET_AVX512DQ && !MEM_P (operands[1])"
>>
>> There is no need for !MEM_P, this will prevent memory operand, which
>> is allowed by constraint "m".
>>
>> +(define_insn "kmovq"
>> + [(set (match_operand:DI 0 "register_operand" "=k,k,km")
>> + (unspec:DI
>> + [(match_operand:DI 1 "nonimmediate_operand" "r,km,k")]
>> + UNSPEC_KMOV))]
>> + "TARGET_AVX512BW && !MEM_P (operands[1])"
>>
>> Operand 0 should have "nonimmediate_operand" predicate. And here you
>> need && !(MEM_P (op0) && MEM_P (op1)) in insn constraint to prevent
>> mem->mem moves.
>
> Changed according your comments and attached.
Still not good.
+(define_insn "kmovd"
+ [(set (match_operand:SI 0 "register_operand" "=k,k")
+ (unspec:SI
+ [(match_operand:SI 1 "nonimmediate_operand" "r,km")]
+ UNSPEC_KMOV))]
+ "TARGET_AVX512BW && !MEM_P (operands[1])"
Remove !MEM_P in the above pattern.
(define_insn "kmovw"
- [(set (match_operand:HI 0 "nonimmediate_operand" "=k,k")
+ [(set (match_operand:HI 0 "register_operand" "=k,k")
(unspec:HI
[(match_operand:HI 1 "nonimmediate_operand" "r,km")]
UNSPEC_KMOV))]
- "!(MEM_P (operands[0]) && MEM_P (operands[1])) && TARGET_AVX512F"
+ "TARGET_AVX512F && !MEM_P (operands[1])"
Also remove !MEM_P here.
+(define_insn "kadd<mode>"
+ [(set (match_operand:SWI1248x 0 "register_operand" "=r,&r,!k")
+ (plus:SWI1248x
+ (not:SWI1248x
+ (match_operand:SWI1248x 1 "register_operand" "r,0,k"))
+ (match_operand:SWI1248x 2 "register_operand" "r,r,k")))
+ (clobber (reg:CC FLAGS_REG))]
+ "TARGET_AVX512F"
+{
+ switch (which_alternative)
+ {
+ case 0:
+ return "add\t{%k2, %k1, %k0|%k0, %k1, %k2}";
+ case 1:
+ return "#";
+ case 2:
+ if (TARGET_AVX512BW && <MODE>mode == DImode)
+ return "kaddq\t{%2, %1, %0|%0, %1, %2}";
+ else if (TARGET_AVX512BW && <MODE>mode == SImode)
+ return "kaddd\t{%2, %1, %0|%0, %1, %2}";
+ else if (TARGET_AVX512DQ && <MODE>mode == QImode)
+ return "kaddb\t{%2, %1, %0|%0, %1, %2}";
+ else
+ return "kaddw\t{%2, %1, %0|%0, %1, %2}";
+
The above pattern is wrong. Is there really a NOT RTX present,
implying effectively a kaddn?
If this is plain add, then you need to change other add patterns, see
how logic patterns are amended with "k" constraint, added pattern
should look like *k<logic><mode> pattern.
(define_insn "kandn<mode>"
- [(set (match_operand:SWI12 0 "register_operand" "=r,&r,!k")
- (and:SWI12
- (not:SWI12
- (match_operand:SWI12 1 "register_operand" "r,0,k"))
- (match_operand:SWI12 2 "register_operand" "r,r,k")))
+ [(set (match_operand:SWI1248x 0 "register_operand" "=r,&r,!k")
+ (and:SWI1248x
+ (not:SWI1248x
+ (match_operand:SWI1248x 1 "register_operand" "r,0,k"))
+ (match_operand:SWI1248x 2 "register_operand" "r,r,k")))
(clobber (reg:CC FLAGS_REG))]
"TARGET_AVX512F"
{
@@ -8319,10 +8358,50 @@
case 1:
return "#";
case 2:
- if (TARGET_AVX512DQ && <MODE>mode == QImode)
+ if (TARGET_AVX512BW && <MODE>mode == DImode)
+ return "kandnq\t{%2, %1, %0|%0, %1, %2}";
+ else if (TARGET_AVX512BW && <MODE>mode == SImode)
+ return "kandnd\t{%2, %1, %0|%0, %1, %2}";
+ else if (TARGET_AVX512DQ && <MODE>mode == QImode)
return "kandnb\t{%2, %1, %0|%0, %1, %2}";
else
return "kandnw\t{%2, %1, %0|%0, %1, %2}";
The above should use SWI1248_AVX512BW mode iterator, see
*k<logic><mode> pattern.
Uros.
