Page MenuHomePhabricator

[CodeGen] Don't combine extract + concat vectors with non-legal types
ClosedPublic

Authored by stuij on Jul 6 2020, 8:11 AM.

Details

Summary

The following combine currently breaks in the DAGCombiner:

extract_vector_elt (concat_vectors v4i16:a, v4i16:b), x
   -> extract_vector_elt a, x

This happens because after we have combined these nodes we have inserted nodes
that use individual instances of the vector element type. In the above example
i16. However this isn't a legal type on all backends, and when the combining pass calls
the legalizer it breaks as it expects types to already be legal. The type legalizer has
already been run, and running it again would make a mess of the nodes.

In the example code at least, the generated code is still efficient after the change.

Diff Detail

Event Timeline

stuij created this revision.Jul 6 2020, 8:11 AM
stuij edited the summary of this revision. (Show Details)Jul 6 2020, 8:19 AM
stuij added reviewers: miyuki, arsenm, dmgreen.
stuij edited the summary of this revision. (Show Details)Jul 6 2020, 8:21 AM
stuij edited the summary of this revision. (Show Details)
stuij edited the summary of this revision. (Show Details)Jul 6 2020, 9:17 AM
miyuki accepted this revision.Tue, Jul 7, 4:23 AM

LGTM. Please wait until Friday before committing, so that others have a chance to share their objections.

This revision is now accepted and ready to land.Tue, Jul 7, 4:23 AM
lebedev.ri requested changes to this revision.Tue, Jul 7, 4:26 AM
lebedev.ri added a subscriber: lebedev.ri.
lebedev.ri added inline comments.
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
17848–17849

!LegalTyepes || TLI.isTypeLegal(

This revision now requires changes to proceed.Tue, Jul 7, 4:26 AM
stuij updated this revision to Diff 276368.Wed, Jul 8, 4:23 AM

addressed review comment

stuij marked 2 inline comments as done.Wed, Jul 8, 4:26 AM
stuij added inline comments.
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
17848–17849

Thanks. Fixed this.

lebedev.ri accepted this revision.Wed, Jul 8, 4:35 AM

Thanks

This revision is now accepted and ready to land.Wed, Jul 8, 4:35 AM
This revision was automatically updated to reflect the committed changes.
stuij marked an inline comment as done.