This is an archive of the discontinued LLVM Phabricator instance.

[X86][DAGCombiner] Teach narrowShuffle to use concat_vectors instead of inserting into undef
ClosedPublic

Authored by craig.topper on Aug 20 2019, 3:58 PM.

Details

Summary

Concat_vectors is more canonical during early DAG combine. For example, its what's used by SelectionDAGBuilder when converting IR shuffles into SelectionDAG shuffles when element counts between inputs and mask don't match. We also have combines in DAGCombiner than can pull concat_vectors through a shuffle. See partitionShuffleOfConcats. So it seems like concat_vectors is a better operation to use here. I had to teach DAGCombiner's SimplifyVBinOp to also handle concat_vectors with undef. I haven't checked yet if we can remove the INSERT_SUBVECTOR version in there or not.

I didn't want to mess with the other caller of getShuffleHalfVectors that's used during shuffle lowering where insert_subvector probably is what we want to produce so I've enabled this via a boolean passed to the function.

Diff Detail

Repository
rL LLVM

Event Timeline

craig.topper created this revision.Aug 20 2019, 3:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 20 2019, 3:58 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
RKSimon added inline comments.Aug 22 2019, 8:23 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
19482 ↗(On Diff #216276)

Not sure if its worth it but you could put the check for getOpcode() == ISD::CONCAT_VECTORS in here as well.

Address review comment

RKSimon accepted this revision.Aug 25 2019, 6:27 AM

LGTM with one optional change

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
19482 ↗(On Diff #216276)

I meant

return Concat.getOpcode() == ISD::CONCAT_VECTORS && std::all_of();
This revision is now accepted and ready to land.Aug 25 2019, 6:27 AM
This revision was automatically updated to reflect the committed changes.