Current custom lowering of truncate vector handles a source of up to 128 bits, but that only uses one of the two shuffle vector operands. Extend it to use both operands to handle 256 bit sources.
Details
- Reviewers
hfinkel nemanjai - Group Reviewers
Restricted Project - Commits
- rGb6d7ed469f2f: [PowerPC] Extend custom lower of vector truncate to handle wider input
Diff Detail
Event Timeline
Is it possible to check the permutation mask in the test case? (I suppose that these are auto-generated tests, but the test coverage it's all that good if we can't check the mask).
Moved the tests to a separate file and added hand-written checks which include the permute control data.
llvm/test/CodeGen/PowerPC/vec-trunc2.ll | ||
---|---|---|
18 ↗ | (On Diff #222174) | Use CHECK-COUNT? |
llvm/lib/Target/PowerPC/PPCISelLowering.cpp | ||
---|---|---|
7166 | I believe DAG.getTargetLoweringInfo() is just *this so we should be able to just call getVectorIdxTy() unqualified. | |
7169 | Isn't SplitVT just SrcVT.getHalfNumVectorElementsVT(*DAG.getContext())? Seems that all of the definitions here can just be something like: EVT VecIdxTy = getVectorIdxTy(DAG.getDataLayout()); EVT SplitVT = N1.getValueType().getHalfNumVectorElementsVT(*DAG.getContext()); unsigned SplitNumElts = SplitVT.getVectorNumElements(); | |
10208–10210 | We check that the number of elements in the wide input is a power of 2 as well as that the target element type width is a power of 2. | |
llvm/test/CodeGen/PowerPC/vec-trunc2.ll | ||
21 ↗ | (On Diff #225056) | Please add a check that shows how we produced v3. Presumably this is a shift from v2? |
Resolve comment.
llvm/lib/Target/PowerPC/PPCISelLowering.cpp | ||
---|---|---|
10208–10210 | Moved checks to lowering function. There was an existing check for source bit width. so 8x24 was skipped.. Added some additional checking. Added an 8x24 test to the test case. |
Other than a couple of minor nits, LGTM. I am so sorry about the delay in reviewing this.
llvm/lib/Target/PowerPC/PPCISelLowering.cpp | ||
---|---|---|
7174 | I realize that we don't need to bitcast Op1 here because it already gets bitcasted below. But I think it is one of those things that a reader looking at this code thinks "Huh? How come only one is bitcasted?" (until they get to the bottom of the function). So my minor nit here would be to just do both bicasts at the same point below unconditionally. Bitcasting to the same type should not produce a new node. | |
llvm/test/CodeGen/PowerPC/vec-trunc2.ll | ||
65 ↗ | (On Diff #279948) | It's bizarre that the script put the checks inside the signature of the function. |
I believe DAG.getTargetLoweringInfo() is just *this so we should be able to just call getVectorIdxTy() unqualified.