This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] remove extract-of-select vector transform
ClosedPublic

Authored by spatel on Sep 18 2017, 4:00 PM.

Details

Summary

The transform to convert an extract-of-a-select-of-vectors was added at:
rL194013

I think most of the motivating cases in that patch (all the tests that still show improvements, but are not changing with this patch) are now handled by other combines.

The diffs we see after removing this transform cause us to avoid increasing the instruction count, so I don't think we want to be doing those transforms as canonicalizations.

The motivation for not turning a vector-select-of-vectors into a scalar operation is shown in PR33301:
https://bugs.llvm.org/show_bug.cgi?id=33301
...in those cases, we'll get vector ops with this patch rather than the vector/scalar mix that we currently see.

Diff Detail

Event Timeline

spatel created this revision.Sep 18 2017, 4:00 PM
efriedma edited edge metadata.
efriedma added a subscriber: nadav.

From https://reviews.llvm.org/D1539:

In D1539#19310, @nadav wrote:

When the elements are extracted from a select, do the select on the extracted scalars from the input.

I am not sure that this is always profitable or that it should be done in InstCombine. It is hard to know at IR-level (during inst-combine) if it is better to extract twice and perform a single select or if it is better to perform an vector select and extract once. I think that this kind of decision should be made in SelectionDAG.

This revision is now accepted and ready to land.Sep 25 2017, 9:23 AM
This revision was automatically updated to reflect the committed changes.