Page MenuHomePhabricator

[SLP] Allow phi node reordering in tryToVectorizeList.
ClosedPublic

Authored by eraman on Apr 13 2017, 4:59 PM.

Details

Summary
In tryToVectorizeList, under a very limited circumstance (when entered from tryToVectorizePair), the values may be reordered (swapped) and the SLP tree is built with the new order. This extends that to the case when starting from phis in vectorizeChainsInBlock when there are exactly two phis. The textual order of phi nodes shouldn't really matter. Without this change, the loop body in the accompnaying test case is fully vectorized when we swap the orde of the phis but not with this order. While this doesn't solve the phi-ordering problem in a general way (for more than 2 phis), this is simple fix that piggybacks on an existing mechanism and is useful in cases like multiplying two complex numbers.

Diff Detail

Repository
rL LLVM

Event Timeline

eraman created this revision.Apr 13 2017, 4:59 PM
mkuper accepted this revision.Apr 17 2017, 7:20 PM

This looks ok, in the sense that it doesn't make things any worse, and will catch more cases - but I think it, unfortunately, doesn't really make things better either.
I don't think we can generalize this (because it blows up exponentially), so eventually we'll have to fix the underlying problems with the SLP "tree".

Anyway, LGTM, with some nits about the test.

test/Transforms/SLPVectorizer/X86/reorder_phi.ll
13 ↗(On Diff #95247)

Can you make the test more explicit, to make sure we vectorize this the way we expect?

46 ↗(On Diff #95247)

No need to have TBAA in the test, I assume?

This revision is now accepted and ready to land.Apr 17 2017, 7:20 PM
eraman updated this revision to Diff 95605.Apr 18 2017, 11:27 AM

Update test case.

eraman marked 2 inline comments as done.Apr 18 2017, 11:27 AM
This revision was automatically updated to reflect the committed changes.