This is an archive of the discontinued LLVM Phabricator instance.

[X86][SSE] Lower 128-bit vectors to SIGN/ZERO_EXTEND_VECTOR_IN_REG ops.
ClosedPublic

Authored by RKSimon on Mar 2 2017, 1:03 PM.

Details

Summary

As described on PR31712, we miss a variety of legalization combines because we lower these to X86ISD::VSEXT/VZEXT despite them having the same functionality. This patch makes 128-bit (SSE41) SIGN/ZERO_EXTEND_VECTOR_IN_REG ops legal, adds the necessary tablegen plumbing and uses a helper 'getExtendInVec' to decide when to use SIGN/ZERO_EXTEND_VECTOR_IN_REG or VSEXT/VZEXT.

We're missing a couple of shuffle combines that will be added in a future patch for review.

Later patches can then support the AVX2 cases as a mixture of SIGN/ZERO_EXTEND and SIGN/ZERO_EXTEND_VECTOR_IN_REG, and then finally deal with the AVX512 cases.

Diff Detail

Repository
rL LLVM

Event Timeline

RKSimon created this revision.Mar 2 2017, 1:03 PM
craig.topper added inline comments.Mar 2 2017, 10:45 PM
include/llvm/Target/TargetSelectionDAG.td
165 ↗(On Diff #90378)

Should this have SDTCisOpSmallerThanOp<1, 0> too?

lib/CodeGen/SelectionDAG/SelectionDAG.cpp
2449 ↗(On Diff #90378)

Why no handling for ISD::SIGN_EXTEND_VECTOR_INREG?

RKSimon added inline comments.Mar 3 2017, 5:50 AM
include/llvm/Target/TargetSelectionDAG.td
165 ↗(On Diff #90378)

Yes I'll add that as well.

lib/CodeGen/SelectionDAG/SelectionDAG.cpp
2449 ↗(On Diff #90378)

We have no current test coverage that requires this - I'd prefer to add it as a followup if possible.

RKSimon updated this revision to Diff 90469.Mar 3 2017, 5:59 AM

Updated based on @craig.topper 's comments

craig.topper accepted this revision.Mar 3 2017, 11:08 AM

LGTM with that one comment.

lib/CodeGen/SelectionDAG/SelectionDAG.cpp
2449 ↗(On Diff #90378)

I'm fine with doing it later. Thanks for adding the TODO.

lib/Target/X86/X86ISelLowering.cpp
17715 ↗(On Diff #90469)

Maybe clarify the comments here that 128-bits can't get here with SSE41. It's otherwise a bit surprising to see an SSE41 check with an assert requiring larger than 128-bit vectors.

This revision is now accepted and ready to land.Mar 3 2017, 11:08 AM
This revision was automatically updated to reflect the committed changes.