[PATCH] D155145: [X86] Add AVX-VNNI-INT16 instructions.

[Anna Thomas via Phabricator via cfe-commits]

anna added a comment.

In D155145#4544068 <https://reviews.llvm.org/D155145#4544068>, @pengfei wrote:

> In D155145#4543326 <https://reviews.llvm.org/D155145#4543326>, @anna wrote:
>
>> We see a crash bisected to this patch about using an illegal instruction. 
>> Here's the CPUInfo for the machine:
>>
>>   CPU info:
>>   current cpu id: 22
>>   total 32(physical cores 16) (assigned logical cores 32) (assigned physical cores 16) (assigned_sockets:2 of 2) (8 cores per cpu, 2 threads per core) family 6 model 45 stepping 7 microcode 0x71a, cmov, cx8, fxsr, mmx, sse, sse2, sse3, ssse3, sse4.1, sse4.2, popcnt, vzeroupper, avx, aes, clmul, ht, tsc, tscinvbit, tscinv, clflush
>>   AvgLoads: 0.30, 0.10, 0.18
>>   CPU Model and flags from /proc/cpuinfo:
>>   model name      : Intel(R) Xeon(R) CPU E5-2650 0 @ 2.00GHz
>>   flags           : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc cpuid aperfmperf pni pclmulqdq dtes64 monitor ds_cpl vmx smx est tm2 ssse3 cx16 xtpr pdcm pcid dca sse4_1 sse4_2 x2apic popcnt tsc_deadline_timer aes xsave avx lahf_lm epb pti ssbd ibrs ibpb stibp tpr_shadow vnmi flexpriority ept vpid xsaveopt dtherm ida arat pln pts md_clear flush_l1d
>>   Online cpus: 0-31
>>   Offline cpus:
>>   BIOS frequency limitation: <Not Available>
>>   Frequency switch latency (ns): 20000
>>   Available cpu frequencies: <Not Available>
>>   Current governor: schedutil
>>   Core performance/turbo boost: <Not Available>
>>
>> I don't see `avxvnniint16` in the flags list nor avx2. So, this (relatively new) instruction shouldn't be generated for this machine. Any ideas on why this might be happening?
>
> As far as I can see from the patch, the only way to generate avxvnniint16 instructions is to call its specific intrinsics explicitly. And we will check compiling options in FE before allowing to call the intrinsics. We do have an optimization to generate vnni instructions without intrinsics, but we haven't extend it to avxvnniint16 so far.
> So I don't know what's wrong in your case, could you provide a reproducer for your problem?

I've investigated what is going on. With this patch, we are now passing in `+avxvnniint16` into machine attributes. With that attribute, we now generate an instruction which is illegal on sandybridge machine:

   0x3013f2af:	jmpq   0x3013f09b
     0x3013f2b4:	mov    %rax,%rdi
     0x3013f2b7:	and    $0xfffffffffffffff0,%rdi
  => 0x3013f2bb:	vpbroadcastd %xmm0,%ymm2
     0x3013f2c0:	vpbroadcastd %xmm1,%ymm3

The instruction `vpbroadcastd %xmm0,%ymm2` requires `AVX2` CPU flag: https://www.felixcloutier.com/x86/vpbroadcast. However, the machine has only AVX flag.

This is the complete mattr generated:

  !3 = !{!"-mattr=-prfchw,-cldemote,+avx,+aes,+sahf,+pclmul,-xop,+crc32,-xsaves,-avx512fp16,-sm4,+sse4.1,-avx512ifma,+xsave,-avx512pf,+sse4.2,-tsxldtrk,-ptwrite,-widekl,-sm3,-invpcid,+64bit,-xsavec,-avx512vpopcntdq,+cmov,-avx512vp2intersect,-avx512cd,-movbe,-avxvnniint8,-avx512er,-amx-int8,-kl,-sha512,-avxvnni,-rtm,-adx,-avx2,-hreset,-movdiri,-serialize,-vpclmulqdq,-avx512vl,-uintr,-clflushopt,-raoint,-cmpccxadd,-bmi,-amx-tile,+sse,-gfni,+avxvnniint16,-amx-fp16,+xsaveopt,-rdrnd,-avx512f,-amx-bf16,-avx512bf16,-avx512vnni,+cx8,-avx512bw,+sse3,-pku,-fsgsbase,-clzero,-mwaitx,-lwp,-lzcnt,-sha,-movdir64b,-wbnoinvd,-enqcmd,-prefetchwt1,-avxneconvert,-tbm,-pconfig,-amx-complex,+ssse3,+cx16,-bmi2,-fma,+popcnt,-avxifma,-f16c,-avx512bitalg,-rdpru,-clwb,+mmx,+sse2,-rdseed,-avx512vbmi2,-prefetchi,-rdpid,-fma4,-avx512vbmi,-shstk,-vaes,-waitpkg,-sgx,+fxsr,-avx512dq,-sse4a"}

I've confirmed if we changed to `-avxvnniint16` we do not generate `vpbroadcastd`.

W.r.t. how we get the machine attributes generated through our front-end:

  if (!sys::getHostCPUFeatures(Features))
        return std::move(mattr);
    
      // Fill mattr with default values.
      mattr.reserve(Features.getNumItems());
      for (auto &I : Features) {
        std::string attr(I.first());
        mattr.emplace_back(std::string(I.second ? "+" : "-") + attr);
      }

So, the problem is in getHostCPUFeatures, possibly this line from the patch : 
`Features["avxvnniint16"] = HasLeaf7Subleaf1 && ((EDX >> 10) & 1) && HasAVXSave;`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155145/new/

https://reviews.llvm.org/D155145