This is an archive of the discontinued LLVM Phabricator instance.

[AVX512] Fix insertelement i1 lowering.
ClosedPublic

Authored by igorb on Aug 10 2016, 2:45 AM.

Details

Summary

[AVX512] Fix insertelement i1 lowering.
1 . Use shuffle to insert element i1 into vector. The previous implementation was incorrect ( dest_bit OR src_bit , it doesn't clear the bit if src_bit=0 )

  1. Improve shuffle i1 vector, use CVT2MASK if supported instead TRUNCATE.

Diff Detail

Repository
rL LLVM

Event Timeline

igorb updated this revision to Diff 67491.Aug 10 2016, 2:45 AM
igorb retitled this revision from to [AVX512] Fix insertelement i1 lowering..
igorb updated this object.
igorb added a reviewer: delena.
igorb set the repository for this revision to rL LLVM.
igorb added a subscriber: llvm-commits.
delena edited edge metadata.Aug 10 2016, 6:42 AM

clear -> clean

lib/Target/X86/X86ISelLowering.cpp
12644 ↗(On Diff #67491)

You can do this if IdxVal !=0 and Vec is defined.
Otherwise it can be done cheaper with shifts and "or"s

igorb updated this revision to Diff 67557.Aug 10 2016, 11:10 AM
igorb edited edge metadata.
igorb marked an inline comment as done.

Thanks for review,
fixed according to comments.

igorb updated this revision to Diff 67659.Aug 11 2016, 1:40 AM
delena added inline comments.Aug 11 2016, 4:51 AM
lib/Target/X86/X86ISelLowering.cpp
12652 ↗(On Diff #67659)

Insertion of one bit into first or last position can be done with two SHIFTs + OR.

12655 ↗(On Diff #67659)

lsb -> the first

12656 ↗(On Diff #67659)

Line alignment.

12664 ↗(On Diff #67659)

Move the bit to the last position inside the vector.

12667 ↗(On Diff #67659)

Clean the last bit in the source vector.

12679 ↗(On Diff #67659)

MaskVec[i] = (i == IdxVal) ? 0 : i+NumElements;

or swap parameters in getVectorShuffle()

delena accepted this revision.Aug 11 2016, 5:58 AM
delena edited edge metadata.

Comments about shuffle are wrong, taking them back.
You can commit after fixing all comments inside the code.

This revision is now accepted and ready to land.Aug 11 2016, 5:58 AM
This revision was automatically updated to reflect the committed changes.