This is an archive of the discontinued LLVM Phabricator instance.

[X86][AVX-512] Allow EVEX encoded instruction selection when available for mul v8i32.
ClosedPublic

Authored by igorb on Apr 30 2017, 6:00 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

igorb created this revision.Apr 30 2017, 6:00 AM
craig.topper added inline comments.Apr 30 2017, 11:41 AM
lib/Target/X86/X86InstrSSE.td
6725 ↗(On Diff #97226)

This looks like a change for VPCMPEQQ to remove NoVLX.

igorb added inline comments.May 1 2017, 12:42 AM
lib/Target/X86/X86InstrSSE.td
6725 ↗(On Diff #97226)

AVX512 version of vpcmpeqq is not equivalent to AVX one. I think it was disabled by mistake.

AVX
Src: (X86pcmpeq:v2i64 VR128:v2i64:$src1, VR128:v2i64:$src2)
Dst: (VPCMPEQQrr:v2i64 VR128:v2i64:$src1, VR128:v2i64:$src2)

AVX512
Src: (X86pcmpeqm:v2i1 VR128X:v2i64:$src1, VR128X:v2i64:$src2)
Dst: (VPCMPEQQZ128rr:v2i1 VR128X:v2i64:$src1, VR128X:v2i64:$src2)

craig.topper edited edge metadata.May 1 2017, 8:33 AM

I'm sure it was a mistake, but I think it should be split from this patch and done as a pre-commit.

test/CodeGen/X86/avx-isa-check.ll
683 ↗(On Diff #97226)

What is this test really testing? Just that isel doesn't fail if you do a v8i32 mul with various isel combinations? It doesn't look like it tests which instruction is emitted. Would this test case even fail if you put it in with no other changes? If that's correct I think we should have a test that checks which instruction is emitted.

igorb updated this revision to Diff 97566.May 3 2017, 1:07 AM
  • Add test, base on encoding.
igorb added a comment.May 3 2017, 1:19 AM

I think we can base the predicate checks on encoding and "EVEX TO VEX Compression". I will pre-commit changes to avx512vl-arith.ll if the approach is acceptable.
In GlobalIsel https://reviews.llvm.org/D32698 i explicit check every instruction selection.

test/CodeGen/X86/avx-isa-check.ll
683 ↗(On Diff #97226)

yes, It only check that isel doesn't fail with various isel combinations.

This revision is now accepted and ready to land.May 3 2017, 8:27 AM
This revision was automatically updated to reflect the committed changes.