Page MenuHomePhabricator

[LLVM][X86] Converting X86 intrinisics movmsk{ps|pd}{|256} and pmovmskb{128|256}
Needs ReviewPublic

Authored by m_zuckerman on Nov 26 2017, 12:39 AM.

Details

Summary

In this patch, I am converting the above intrinsic into generic IR. This allows us to do further optimization on the above intrinsic. Besides of the above, we added also a new instcombine that combines ExtractElementInst with bitcast into shuffle instruction. This folding fixes an issue of inserting an avx2's intrinsics as input into avx512's intrinsics.

The patch is a part of two patches: clang side and this LLVM side.
clang side: https://reviews.llvm.org/D40465

Diff Detail

Event Timeline

m_zuckerman created this revision.Nov 26 2017, 12:39 AM
m_zuckerman edited the summary of this revision. (Show Details)Nov 26 2017, 3:41 AM
guyblank added inline comments.Dec 14 2017, 1:23 AM
lib/IR/AutoUpgrade.cpp
206

what's this one? i don't see it in the removed intrinsics

208

this one also

919

typo EmitX86Msak -> EmitX86Mask

932

typo, same as above

1314

same one from above

lib/Target/X86/X86ISelLowering.cpp
35708

should the combines be separate commits?
also they need tests

lib/Transforms/InstCombine/InstCombineCasts.cpp
2044 ↗(On Diff #124275)

separate patch + tests

test/CodeGen/X86/avx2-intrinsics-x86.ll
1 ↗(On Diff #124275)

can you regenerate and commit this test, there's a lot of noise here.

test/CodeGen/X86/movmsk.ll
136

this doesn't seem right, what happened here?

test/Transforms/InstCombine/X86/x86-movmsk.ll
1

no reason to keep this test anymore, you've removed the combines it is testing.

m_zuckerman marked 4 inline comments as done.
m_zuckerman added inline comments.Dec 17 2017, 6:23 AM
lib/Target/X86/X86ISelLowering.cpp
35708

All the combines are important for the test. Since we deleted intrinsics name, the front end IR doesn't aware of names. And so instcombinecall cant works for them. For satisfying the tests, we had to add new combines that replace the old (instcombinecall).

All the combines are already covered by original tests.

lib/Transforms/InstCombine/InstCombineCasts.cpp
2044 ↗(On Diff #124275)

I agree with you

m_zuckerman updated this revision to Diff 127278.
m_zuckerman added inline comments.Dec 17 2017, 6:27 AM
test/Transforms/InstCombine/X86/x86-movmsk.ll
1

The first case is not cover it (MMX)

you've uploaded the clang patch instead of the llvm one.

igorb resigned from this revision.Aug 14 2018, 1:57 AM