On Thu, Nov 10, 2016 at 6:18 PM, Andrew Senkevich <andrew.n.senkev...@gmail.com> wrote: > 2016-11-10 19:36 GMT+03:00 Jakub Jelinek <ja...@redhat.com>: >> On Thu, Nov 10, 2016 at 07:27:00PM +0300, Andrew Senkevich wrote: >>> Hi, >>> >>> this patch enabled AVX512_4FMAPS and AVX512_4VNNIW instructions. >>> >>> It requires additional patch for register allocator from Vladimir >>> Makarov to be committed before. >> >> Your MUA ate tabs (and in the ChangeLog you're using spaces instead of >> tabs), can you repost as attachment or configure your MUA not to do this? >> >> Just a couple of random nits follow: >> >>> * gcc.target/i386/sse-12.c: Add -mavx5124fmaddps. >> >> This mentions an option that doesn't exist, is that s/dd// ? > > Yes. > Attached fixed version.
A couple of questions and comments below. You are introducing flag2 ABI option flags. There are no tests for corresponding __target__ attribute, please add some tests, similar to gcc.target/i386/funcspec-?.c. These can be in a follow-up patch. Please add new option to g++.dg/other/i386-{2,3}.C tests. These are like gcc.target/i386/sse-{22,23}.c for c++. Also, I guess we want to support these new options with __builtin_cpu_supports. Please add this functionality in a follow-up patch. +(define_register_constraint "h" "TARGET_AVX512F ? MOD4_SSE_REGS : NO_REGS" + "Any EVEX encodable SSE register, which has number factor of four.") + No, we are extremely low on a single-letter constraints. We will use these for possible future new register sets. Use Yv or something similar instead. +//additional structure for isa flags Please use c comments throughout the patch. @@ -1465,11 +1472,14 @@ enum reg_class { 0x11ffff, 0x1fe0, 0x0 }, /* FLOAT_INT_REGS */ \ { 0x1ff100ff,0xffffffe0, 0x1f }, /* INT_SSE_REGS */ \ { 0x1ff1ffff,0xffffffe0, 0x1f }, /* FLOAT_INT_SSE_REGS */ \ - { 0x0, 0x0, 0x1fc0 }, /* MASK_EVEX_REGS */ \ + { 0x0, 0x0, 0x1fc0 }, /* MASK_EVEX_REGS */ \ { 0x0, 0x0, 0x1fe0 }, /* MASK_REGS */ \ -{ 0xffffffff,0xffffffff,0x1ffff } \ +{ 0x1fe00000,0xffffe000, 0x1f }, /* MOD4_SSE_REGS */ \ +{ 0xffffffff,0xffffffff,0x1ffff } \ } +/* { 0x02200000,0x22222000, 0x02 },*/ /* MOD4_SSE_REGS */ + Please remove commented out code. Also, please fix whitespace at the new entry. +mavx5124fmaps +Target Report Mask(ISA_AVX5124FMAPS) Var(ix86_isa_flags2) Save +Support MMX, SSE, SSE2, SSE3, SSSE3, SSE4.1, SSE4.2, AVX, AVX2 and AVX512F and AVX5124FMAPS built-in functions and code generation. + +mavx5124vnniw +Target Report Mask(ISA_AVX5124VNNIW) Var(ix86_isa_flags2) Save +Support MMX, SSE, SSE2, SSE3, SSSE3, SSE4.1, SSE4.2, AVX, AVX2 and AVX512F and AVX5124VNNIW built-in functions and code generation. Too much "and"s in the description. --- a/gcc/genmodes.c +++ b/gcc/genmodes.c --- a/gcc/init-regs.c +++ b/gcc/init-regs.c --- a/gcc/machmode.h +++ b/gcc/machmode.h These are middle-end changes, you will need a separate review for these. The x86 part of the patch is OK with the above changes and additional target attribute test for flags2 ISA features.. Uros.