This is an archive of the discontinued LLVM Phabricator instance.

[X86] Fix a bug with i386 subtarget in LowerCONCAT_VECTORSvXi1 func
ClosedPublic

Authored by uriel.k on Oct 9 2017, 8:20 AM.

Details

Summary

Previously, did not consider i386 subtargets which led to a crash in legalization stage.

This is required before adding support for X86TESTM and X86TESTNM
for also eliminating zeroing upper bits redundant instructions.

Diff Detail

Event Timeline

uriel.k created this revision.Oct 9 2017, 8:20 AM
This revision is now accepted and ready to land.Oct 9 2017, 12:10 PM
RKSimon added a subscriber: RKSimon.

Test case?

lib/Target/X86/X86ISelLowering.cpp
8032

Surely you could just use DAG.getIntPtrConstant(0, dl) ?

craig.topper added inline comments.Oct 9 2017, 1:38 PM
lib/Target/X86/X86ISelLowering.cpp
8032

Should this just be calling getZeroVector?

m_zuckerman added a comment.EditedOct 9 2017, 11:33 PM

Test case?

This bug is covered by a test in check-llvm and after that @uriel.k will commit the next patch the case will be covered.

aymanmus added inline comments.Oct 10 2017, 12:30 AM
lib/Target/X86/X86ISelLowering.cpp
8032

It's possible. But since the ZeroC has an additional use (cannot be removed), it will end up the same.

uriel.k updated this revision to Diff 118326.Oct 10 2017, 3:33 AM

Changed function to be DAG.getIntPtrConstant(0, dl) instead.

Test case is not possible right now as it crashes only after another patch that will add TESTM to
the list of instructions that zeroing the upper bits by default (much like PCMPEQ).

RKSimon accepted this revision.Oct 10 2017, 4:16 AM

LGTM with one minor

lib/Target/X86/X86ISelLowering.cpp
8032

Please just use getZeroVector() for consistency.

uriel.k closed this revision.Oct 10 2017, 6:58 AM

Committed in rL315311. Sorry for the poor commit message, added a suitable message in comment in https://reviews.llvm.org/rL315311