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. -- WBR, Andrew
add_k-mask_intrinsics_11.11_1.patch
Description: Binary data
