Page MenuHomePhabricator

[DAGCombine]Expand usage of CreateBuildVecShuffle to make full use of vector ops
ClosedPublic

Authored by XinWang10 on Dec 8 2022, 5:40 PM.

Details

Summary

Now, when llc encounters the case that contains a lot of
extract_vector_elt and a BUILD_VECTOR, it will replace these to
vector_shuffle to decrease the size of code, the actions are done in
createBuildVecShuffle in DAGCombiner.cpp, but now the code cannot handle
the case that the size of source vector reg is more than twice the dest
size.

Diff Detail

Event Timeline

XinWang10 created this revision.Dec 8 2022, 5:40 PM
XinWang10 requested review of this revision.Dec 8 2022, 5:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 8 2022, 5:40 PM

Now, when llc encounters the case that contains a lot of extract_vector_elt and a BUILD_VECTOR, it will replace these to vector_shuffle to decrease the size of code, the actions are done in createBuildVecShuffle in DAGCombiner.cpp, but now the code cannot handle the case that the size of source vector reg is more than twice the dest size.

Better to limit column width to 80.

Now, when llc encounters the case that contains a lot of
extract_vector_elt and a BUILD_VECTOR, it will replace these to
vector_shuffle to decrease the size of code, the actions are done in
createBuildVecShuffle in DAGCombiner.cpp, but now the code cannot handle
the case that the size of source vector reg is more than twice the dest
size.

XinWang10 edited the summary of this revision. (Show Details)Dec 8 2022, 6:17 PM
XinWang10 edited the summary of this revision. (Show Details)

Now, when llc encounters the case that contains a lot of extract_vector_elt and a BUILD_VECTOR, it will replace these to vector_shuffle to decrease the size of code, the actions are done in createBuildVecShuffle in DAGCombiner.cpp, but now the code cannot handle the case that the size of source vector reg is more than twice the dest size.

Better to limit column width to 80.

Now, when llc encounters the case that contains a lot of
extract_vector_elt and a BUILD_VECTOR, it will replace these to
vector_shuffle to decrease the size of code, the actions are done in
createBuildVecShuffle in DAGCombiner.cpp, but now the code cannot handle
the case that the size of source vector reg is more than twice the dest
size.

Got it.

XinWang10 retitled this revision from Expand usage of CreateBuildVecShuffle to make full use of vector ops to [DAGCombine]Expand usage of CreateBuildVecShuffle to make full use of vector ops.Dec 8 2022, 10:23 PM
XinWang10 updated this revision to Diff 481527.Dec 8 2022, 11:01 PM

adjust format

XinWang10 updated this revision to Diff 481531.Dec 8 2022, 11:48 PM

fail to apply patch

XinWang10 updated this revision to Diff 481535.Dec 9 2022, 12:18 AM

update patch

RKSimon added inline comments.Dec 9 2022, 4:15 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
21203

This comment looks like it was copy + pasted?

pengfei accepted this revision.Dec 20 2022, 6:47 PM

LGTM.

This revision is now accepted and ready to land.Dec 20 2022, 6:47 PM

FWIW, i've recently looked at this method, and, irrespectively of this particular patch,
indeed it misses a quite big number of opportunities, but it also has a very high impact
on the produced DAG, so coming up with the right preconditions is not easy. Alternatively,
we need a significant amount of yak shawing to deal with the new patterns.

FWIW, i've recently looked at this method, and, irrespectively of this particular patch,
indeed it misses a quite big number of opportunities, but it also has a very high impact
on the produced DAG, so coming up with the right preconditions is not easy. Alternatively,
we need a significant amount of yak shawing to deal with the new patterns.

It would be great if we can figure out with meticulous patterns. But I think it's still fine to land this as a small step forward.

This revision was landed with ongoing or failed builds.Sun, Jan 22, 7:45 PM
This revision was automatically updated to reflect the committed changes.