This is an archive of the discontinued LLVM Phabricator instance.

[SelectionDAG][X86] Don't use SEXTLOAD for promoting masked loads in the type legalizer
ClosedPublic

Authored by craig.topper on Jan 24 2019, 1:32 PM.

Details

Summary

I'm not sure why we were using SEXTLOAD. EXTLOAD seems more appropriate since we don't care about the upper bits.

This patch changes this and then modifies the X86 post legalization combine to emit a extending shuffle instead of a sign_extend_vector_inreg. Could maybe use an any_extend_vector_inreg, but I just did what we already do in LowerLoad. I think we can actually get rid of this code entirely if we switch to -x86-experimental-vector-widening-legalization.

On AVX512 targets I think we might be able to use a masked vpmovzx and not have to expand this at all.

Diff Detail

Event Timeline

craig.topper created this revision.Jan 24 2019, 1:32 PM
RKSimon accepted this revision.Jan 25 2019, 12:43 AM

LGTM - thanks

This revision is now accepted and ready to land.Jan 25 2019, 12:43 AM
This revision was automatically updated to reflect the committed changes.

I couldn't make sense of the error, but I've reverted this patch to see if it fixes it.

@stella.stamenova I don't think reverting mine fixed it. I notice this commit was also around the same time, but isn't logged. http://llvm.org/viewvc/llvm-project?revision=352254&view=revision Does the bot also build lld and use it?

@stella.stamenova I don't think reverting mine fixed it. I notice this commit was also around the same time, but isn't logged. http://llvm.org/viewvc/llvm-project?revision=352254&view=revision Does the bot also build lld and use it?

@craig.topper Yes, the Buildbot builds and uses lld as well. I am not sure why this change wasn't logged, but it could be the reason for the break also (and it looks likely).