This is an archive of the discontinued LLVM Phabricator instance.

[INSTCOMBINE][X86][SSE] Replace sign/zero extension intrinsics with native IR
ClosedPublic

Authored by RKSimon on Jul 25 2015, 8:07 AM.

Details

Summary

Now that we are generating sane codegen for vector sext/zext nodes on SSE targets, this patch uses instcombine to replace the SSE41/AVX2 pmovsx and pmovzx intrinsics with the equivalent native IR code.

I have investigated removing these intrinsics completely, but we are still up against the issue of debug/-O0 codegen. At present this would be doable for some of the AVX2 intrinsics only (the ones that extend the entire xmm to a ymm) - if people think this is worthwhile I can provide patches that update the avx2 headers to use __builtin_convertvector where possible and remove those specific intrinsics entirely but I thought it best to keep them all for now and include them in this instcombine patch.

Diff Detail

Repository
rL LLVM

Event Timeline

RKSimon updated this revision to Diff 30635.Jul 25 2015, 8:07 AM
RKSimon retitled this revision from to [INSTCOMBINE][X86][SSE] Replace sign/zero extension intrinsics with native IR.
RKSimon updated this object.
RKSimon set the repository for this revision to rL LLVM.
RKSimon added a subscriber: llvm-commits.
spatel edited edge metadata.Jul 25 2015, 9:23 AM
spatel added subscribers: ab, probinson, echristo.

LGTM; see inline comments for nits.

But as Paul suggested recently, we should decide how to handle these in general since we have a mishmash of implementations...
Then start the drudge work of fixing the hundreds of intrinsics that we decide are done wrong. :(

Reference:
http://reviews.llvm.org/D10555
https://llvm.org/bugs/show_bug.cgi?id=24125 and related bugs

lib/Transforms/InstCombine/InstCombineCalls.cpp
208 ↗(On Diff #30635)

Remove 'sign' from comment since this is a general extend now?

215–217 ↗(On Diff #30635)

Can fold these 2 lines together; no need for EX temp.

RKSimon updated this revision to Diff 30636.Jul 25 2015, 9:55 AM
RKSimon edited edge metadata.

Updated based on Sanjay's feedback

spatel accepted this revision.Jul 27 2015, 8:06 AM
spatel edited edge metadata.

Forgot to set the 'Accept' action here on Phab.

I don't see much downside to checking this in, regardless of the outcome of any future decision about how to best handle intrinsics in general.

This revision is now accepted and ready to land.Jul 27 2015, 8:06 AM
This revision was automatically updated to reflect the committed changes.