On Fri, Aug 21, 2020 at 6:29 PM Hongtao Liu <crazy...@gmail.com> wrote: > > On Fri, Aug 21, 2020 at 11:50 PM Uros Bizjak <ubiz...@gmail.com> wrote: > > > > On Fri, Aug 21, 2020 at 5:41 PM Hongtao Liu <crazy...@gmail.com> wrote: > > > > > > On Fri, Aug 21, 2020 at 9:15 PM Uros Bizjak <ubiz...@gmail.com> wrote: > > > > > > > > > > > gcc/ > > > > > > > PR target/88808 > > > > > > > * config/i386/i386.c (ix86_preferred_reload_class): Allow > > > > > > > QImode data go into mask registers. > > > > > > > * config/i386/i386.md: (*movhi_internal): Adjust > > > > > > > constraints > > > > > > > for mask registers. > > > > > > > (*movqi_internal): Ditto. > > > > > > > (*anddi_1): Support mask register operations > > > > > > > (*and<mode>_1): Ditto. > > > > > > > (*andqi_1): Ditto. > > > > > > > (*andn<mode>_1): Ditto. > > > > > > > (*<code><mode>_1): Ditto. > > > > > > > (*<code>qi_1): Ditto. > > > > > > > (*one_cmpl<mode>2_1): Ditto. > > > > > > > (*one_cmplsi2_1_zext): Ditto. > > > > > > > (*one_cmplqi2_1): Ditto. > > > > > > > (define_peephole2): Move constant 0/-1 directly into mask > > > > > > > registers. > > > > > > > * config/i386/predicates.md (mask_reg_operand): New > > > > > > > predicate. > > > > > > > * config/i386/sse.md (define_split): Add post-reload > > > > > > > splitters > > > > > > > that would convert "generic" patterns to mask patterns. > > > > > > > (*knotsi_1_zext): New define_insn. > > > > > > > > > > > > > > gcc/testsuite/ > > > > > > > * gcc.target/i386/bitwise_mask_op-1.c: New test. > > > > > > > * gcc.target/i386/bitwise_mask_op-2.c: New test. > > > > > > > * gcc.target/i386/bitwise_mask_op-3.c: New test. > > > > > > > * gcc.target/i386/avx512bw-pr88465.c: New testcase. > > > > > > > * gcc.target/i386/avx512bw-kunpckwd-1.c: Adjust testcase. > > > > > > > * gcc.target/i386/avx512bw-kunpckwd-3.c: Ditto. > > > > > > > * gcc.target/i386/avx512dq-kmovb-5.c: Ditto. > > > > > > > * gcc.target/i386/avx512f-kmovw-5.c: Ditto. > > > > > > > > > > > > A little nit, please put new splitters after the instruction > > > > > > pattern. > > > > > > > > > > > > OK for the whole patch set with the above change, > > > > > > > > > > > > > > > > Yes, thanks for the review. > > > > > > > > Please note that your patch introduces several testsuite fails with > > > > -m32: > > > > > > > > gcc -O2 -mavx512bitalg -mavx512bw -m32 -g avx512bitalg-vpopcntb-1.c > > > > > > > > > > I can't reproduce this failure. > > > > Because you are running it on AVX512 enabled target. > > > > > > Program received signal SIGILL, Illegal instruction. > > > > 0x080490ac in __get_cpuid_count (__edx=<synthetic pointer>, > > > > __ecx=<synthetic pointer>, __ebx=<synthetic pointer>, __eax=<synthetic > > > > pointer>, > > > > __subleaf=0, __leaf=7) at > > > > /hdd/uros/gcc-build-fast/gcc/include/cpuid.h:316 > > > > 316 __cpuid_count (__leaf, __subleaf, *__eax, *__ebx, *__ecx, > > > > *__edx); > > > > > > > > 0x080490a3 <+51>: cpuid > > > > 0x080490a5 <+53>: mov $0x1,%eax > > > > 0x080490aa <+58>: mov %ecx,%esi > > > > => 0x080490ac <+60>: kmovd %ebx,%k0 > > > > 0x080490b0 <+64>: mov %edi,%ecx > > > > 0x080490b2 <+66>: mov %edi,%ebx > > > > > > > > kmov insn is generated for __cpuid_count function, where the binary > > > > determines, if the new instructions are supported. The binary will > > > > crash in the detection code if the processor lacks AVX512 > > > > instructions. > > > > > > > > > > IMHO, the testcase shouldn't be run on processors without AVX512BW. > > > > No, it could run, because it checks for AVX512BW at runtime. > > > > Got it. > > > > Because in avx512bitalg-vpopcntb-1.c, there's /* > > > dg-require-effective-target avx512bw } */. > > > > This is to check the toolchain for support. > > > > > what's the version of your assembler？ > > > > GNU assembler version 2.34-4.fc32 > > > > If assembler supports avx512bw, but processor not, the test would pass > condition `dg-require-effective-target avx512bw` and be runned. > then crashed for illegal instruction.
Please look at avx512-check.h. This is where main function lives. > > Please add something like > > X86_TUNE_INTER_UNIT_MOVES_FROM_MASK/X86_TUNE_INTER_UNIT_MOVES_TO_MASK > > and enable them only for m_CORE_AVX512 (or perhaps m_INTEL). > > > > Handle this in inline_secondary_memory_needed to reject direct moves > > for all other targets. This should disable direct moves for generic > > targets. > > > > Yes, I'll add it. Thanks. Uros.
