This is an archive of the discontinued LLVM Phabricator instance.

[DAG] Extend visitSCALAR_TO_VECTOR optimization to truncated vector.
ClosedPublic

Authored by niravd on Jul 18 2017, 8:12 AM.

Event Timeline

niravd created this revision.Jul 18 2017, 8:12 AM
efriedma added inline comments.Jul 18 2017, 2:22 PM
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
15751–15753

The only way for VT.getScalarType() == InVal.getValueType() to be false is if legalization promoted it to a larger integer type... and I can't why that should block the transform (the extra bits get discarded anyway).

15757

Do you need to check that the element type matches? And in that case, isn't the "VT.getVectorNumElements() == InVecT.getVectorNumElements()" check redundant (since it implies VT == InVecT)?

niravd updated this revision to Diff 108086.Jul 25 2017, 8:23 AM

Update to weaken type restrictions

efriedma added inline comments.Jul 25 2017, 1:19 PM
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
15767

I think you need to verify that VT and InVecT have the same element type here?

niravd updated this revision to Diff 108508.Jul 27 2017, 11:28 AM
niravd marked an inline comment as done.

Add type check in truncate case.

efriedma added inline comments.Jul 27 2017, 11:33 AM
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
15767

"VT.getVectorNumElements() != InVecT.getVectorNumElements()" check is redundant.

niravd updated this revision to Diff 108937.Jul 31 2017, 9:04 AM

Remove redundant check

niravd updated this revision to Diff 108939.Jul 31 2017, 9:10 AM

Unfold unrelated commit.

efriedma accepted this revision.Jul 31 2017, 12:37 PM

LGTM

lib/CodeGen/SelectionDAG/DAGCombiner.cpp
15766

I would probably rearrange this to move the element type check into the outer if statement, for the sake of clarity... but either way is fine, I guess.

This revision is now accepted and ready to land.Jul 31 2017, 12:37 PM
This revision was automatically updated to reflect the committed changes.
niravd reopened this revision.Aug 1 2017, 11:13 AM

Looks like this is raising an assertion. Reopening until I can look at it again.

This revision is now accepted and ready to land.Aug 1 2017, 11:13 AM
This revision was automatically updated to reflect the committed changes.