This is an archive of the discontinued LLVM Phabricator instance.

[X86] Don't base domain decisions on VEXTRACTF128/VINSERTF128 if only AVX1 is available.
ClosedPublic

Authored by craig.topper on Jan 31 2017, 9:18 PM.

Details

Reviewers
RKSimon
zvi
delena
Summary

Seems the execution dependency pass likes to use FP instructions when most of the consuming code is integer if a vextractf128 instruction produced the register. Without AVX2 we don't have the corresponding integer instruction available.

This patch suppresses the domain on these instructions to GenericDomain if AVX2 is not supported so that they are ignored by domain fixing. If AVX2 is supported we'll report the correct domain and allow them to switch between integer and fp.

Overall I think this produces better results in the modified test cases.

Diff Detail

Event Timeline

craig.topper created this revision.Jan 31 2017, 9:18 PM
delena added inline comments.Feb 9 2017, 12:28 AM
lib/Target/X86/X86InstrInfo.cpp
8969

I don't think that moving instructions mapping from tables to "switch" is a good idea.

Update to not use a switch.

RKSimon accepted this revision.Feb 10 2017, 6:07 AM

LGTM

lib/Target/X86/X86InstrInfo.cpp
8698

Very minor - but please can you put this table below ReplaceableInstrsAVX2 to match evaluation order

test/CodeGen/X86/x86-interleaved-access.ll
78

Annoying but not a real problem.

This revision is now accepted and ready to land.Feb 10 2017, 6:07 AM
craig.topper added inline comments.Feb 10 2017, 9:28 PM
test/CodeGen/X86/x86-interleaved-access.ll
78

We should really look into shrinking those loads

RKSimon added inline comments.Feb 11 2017, 5:51 AM
test/CodeGen/X86/x86-interleaved-access.ll
78

Agreed, I'm all for splitting ymm (+ zmm?) loads if the only thing that happens to them is some/all their subvectors get extracted - its a definite win for Jaguar and probably all AVX1 targets - I don't think its even necessary for performance to ensure that the split loads fold? Element extraction is a bit more tricky - there are cases where its useful.

craig.topper closed this revision.Feb 13 2017, 8:32 PM

I lost the differential line from the commit message somehow. Closing as fixed in r294824.