This is an archive of the discontinued LLVM Phabricator instance.

[X86][SSE] Improve support for 128-bit vector sign extension
ClosedPublic

Authored by RKSimon on May 19 2015, 1:27 AM.

Details

Summary

This patch improves support for sign extension of the lower lanes of vectors of integers by making use of the SSE41 pmovsx* sign extension instructions where possible, and optimizing the sign extension by shifts on pre-SSE41 targets (avoiding the use of i64 arithmetic shifts which require scalarization).

It converts SIGN_EXTEND nodes to SIGN_EXTEND_VECTOR_INREG where necessary, which more closely match the pmovsx* instruction than the default approach of using SIGN_EXTEND_INREG which splits the operation (into an ANY_EXTEND lowered to a shuffle followed by shifts) making instruction matching difficult during lowering. Necessary support for SIGN_EXTEND_VECTOR_INREG has been added to the DAGCombiner.

Diff Detail

Repository
rL LLVM

Event Timeline

RKSimon updated this revision to Diff 26039.May 19 2015, 1:27 AM
RKSimon retitled this revision from to [X86][SSE] Improve support for 128-bit vector sign extension.
RKSimon updated this object.
RKSimon edited the test plan for this revision. (Show Details)
RKSimon added reviewers: qcolombet, delena, andreadb.
RKSimon set the repository for this revision to rL LLVM.
RKSimon added a subscriber: Unknown Object (MLST).
delena added inline comments.May 19 2015, 2:08 AM
lib/Target/X86/X86ISelLowering.cpp
23700 ↗(On Diff #26039)

Usually, we do not work with illegal types here.
SIGN_EXTEND_INREG will be created by type legalizer for v2i32.
The same for other types.

RKSimon updated this revision to Diff 26047.May 19 2015, 4:02 AM

Thanks Elena - I've relaxed the types test to just check the input/output scalar types are the correct sizes. Notably we have i1 types coming through that this code shouldn't attempt to deal with.

delena added inline comments.May 19 2015, 5:37 AM
lib/Target/X86/X86ISelLowering.cpp
23687 ↗(On Diff #26047)

I still don't understand why it should be done before type legalizer. You don't want to deal with 3 x i8 or 6 x i32.
What information disappears after type legalizer?

RKSimon added inline comments.May 19 2015, 6:56 AM
lib/Target/X86/X86ISelLowering.cpp
23687 ↗(On Diff #26047)

What happens is that the legalizer promotes the SIGN_EXTEND nodes to a ANY_EXTEND / SIGN_EXTEND_INREG pair (see PromoteIntOp_SIGN_EXTEND) - after which its much harder to match these again for lowering ;-(

I can place the new code before the (!DCI.isBeforeLegalizeOps()) early exit but it won't hit.

delena edited edge metadata.May 20 2015, 6:21 AM

Ok. I don't have more comments, you can commit.
Thanks.

This revision was automatically updated to reflect the committed changes.