On Tue, Nov 7, 2017 at 11:47 AM, Jakub Jelinek <ja...@redhat.com> wrote:
> On Tue, Nov 07, 2017 at 11:21:15AM +0100, Uros Bizjak wrote:
>> On Tue, Nov 7, 2017 at 11:10 AM, Jakub Jelinek <ja...@redhat.com> wrote:
>>
>> >> > Isn't it sufficient that moves disparage slightly the k alternatives?
>> >> > Or are you worried about the case where the pseudo would need to be
>> >> > spilled
>> >> > and LRA would choose to reload it into a %kN register?
>> >>
>> >> The later... Perhaps it is possible to prevent unwanted reloads with
>> >> ?k (or even ??k, we already have ?m<r>, IDK). I have used *k in
>> >> zero-extend patterns to prevent unwanted reloads, which is perhaps too
>> >> strong, but we have ree pass that eliminates unwanted moves in this
>> >> case (*k in move patterns is handled by other regmove elimination
>> >> passes).
>> >
>> > ?k gives the same IRA decisions as *k, in both cases it considers
>> > GENERAL_REGS as expensive as MASK_EVEX_REGS and picks up GENERAL_REGS
>> > rather than MASK_EVEX_REGS for the pseudo, which results in
>> > kmovb %k1, %eax; test.
>>
>> Can we use some of the new RA modifiers here?
>
> So, for the testcase in question, GENERAL_REGS have cost 2000:
> k works (MASK_EVEX_REGS cost 0)
> *k doesn't work, IRA chooses GENERAL_REGS (MASK_EVEX_REGS cost 2000)
> ?k doesn't work, IRA chooses GENERAL_REGS (MASK_EVEX_REGS cost 2000)
> !k works (MASK_EVEX_REGS cost 0)
> ^k doesn't work, IRA chooses GENERAL_REGS (MASK_EVEX_REGS cost 2000)
> $k works (MASK_EVEX_REGS cost 0)
>
>> --cut here--
>> '?'
>> Disparage slightly the alternative that the '?' appears in, as a
>> choice when no alternative applies exactly. The compiler regards
>> this alternative as one unit more costly for each '?' that appears
>> in it.
>>
>> '!'
>> Disparage severely the alternative that the '!' appears in. This
>> alternative can still be used if it fits without reloading, but if
>> reloading is needed, some other alternative will be used.
>>
>> '^'
>> This constraint is analogous to '?' but it disparages slightly the
>> alternative only if the operand with the '^' needs a reload.
>>
>> '$'
>> This constraint is analogous to '!' but it disparages severely the
>> alternative only if the operand with the '$' needs a reload.
>> --cut here--
>>
>> '$' looks promising to prevent unwanted reloads to kN register.
>
> So, ok for trunk if it passes bootstrap/regtest?
>
> 2017-11-07 Jakub Jelinek <ja...@redhat.com>
>
> PR target/82855
> * config/i386/i386.md (SWI1248_AVX512BWDQ2_64): New mode iterator.
> (*cmp<mode>_ccz_1): New insn with $k alternative.
>
> * gcc.target/i386/avx512dq-pr82855.c: New test.
Hopefully OK.
Thanks,
Uros.
> --- gcc/config/i386/i386.md.jj 2017-11-06 17:23:10.674996727 +0100
> +++ gcc/config/i386/i386.md 2017-11-07 11:44:40.081328781 +0100
> @@ -1275,6 +1275,26 @@ (define_expand "cmp<mode>_1"
> (compare:CC (match_operand:SWI48 0 "nonimmediate_operand")
> (match_operand:SWI48 1 "<general_operand>")))])
>
> +(define_mode_iterator SWI1248_AVX512BWDQ2_64
> + [(QI "TARGET_AVX512DQ") (HI "TARGET_AVX512DQ")
> + (SI "TARGET_AVX512BW") (DI "TARGET_AVX512BW && TARGET_64BIT")])
> +
> +(define_insn "*cmp<mode>_ccz_1"
> + [(set (reg FLAGS_REG)
> + (compare (match_operand:SWI1248_AVX512BWDQ2_64 0
> + "nonimmediate_operand" "<r>,?m<r>,$k")
> + (match_operand:SWI1248_AVX512BWDQ2_64 1 "const0_operand")))]
> + "ix86_match_ccmode (insn, CCZmode)"
> + "@
> + test{<imodesuffix>}\t%0, %0
> + cmp{<imodesuffix>}\t{%1, %0|%0, %1}
> + ktest<mskmodesuffix>\t%0, %0"
> + [(set_attr "type" "test,icmp,msklog")
> + (set_attr "length_immediate" "0,1,*")
> + (set_attr "modrm_class" "op0,unknown,*")
> + (set_attr "prefix" "*,*,vex")
> + (set_attr "mode" "<MODE>")])
> +
> (define_insn "*cmp<mode>_ccno_1"
> [(set (reg FLAGS_REG)
> (compare (match_operand:SWI 0 "nonimmediate_operand" "<r>,?m<r>")
> --- gcc/testsuite/gcc.target/i386/avx512dq-pr82855.c.jj 2017-11-07
> 08:56:02.910321163 +0100
> +++ gcc/testsuite/gcc.target/i386/avx512dq-pr82855.c 2017-11-07
> 08:56:02.910321163 +0100
> @@ -0,0 +1,14 @@
> +/* PR target/82855 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -mavx512vl -mavx512dq" } */
> +/* { dg-final { scan-assembler {\mktestb\M} } } */
> +
> +#include <immintrin.h>
> +
> +int
> +foo (const __m256i *ptr)
> +{
> + __m256i v = _mm256_loadu_si256 (ptr);
> + __mmask8 m = _mm256_cmpeq_epi32_mask (v, _mm256_setzero_si256 ());
> + return 0 == m;
> +}
>
>
> Jakub
