Page MenuHomePhabricator

[LV] Move vector induction update to end of latch
ClosedPublic

Authored by mssimpso on Jul 15 2016, 10:56 AM.

Details

Summary

This patch moves the update instruction for vectorized integer induction phi nodes to the end of the latch block. This ensures consistent placement of all induction updates despite the kind of induction we create (scalar, splat vector, or vector phi).

Brief background:

I was comparing the IR, post-instcombine, we generate using the vector splat and vector phi methods of widening induction variables. The vector splat method keeps the original update we create at the end of the latch:

i = phi [0, pre-header], [i.next, latch]
...
i.next = i + 1 
cmp i.next
br

whereas the vector phi method was generating code that simplifies to:

i = phi [0, pre-header], [i.next, latch]
i.next = i + 1 
...
cmp i.next
br

with the update at the top of the header. This is because the original scalar update was being replaced by the new update during simplification. This is not a huge deal, but this patch ensures that for many cases, both methods can simplify to the same IR.

Let me know what you think.

Diff Detail

Repository
rL LLVM

Event Timeline

mssimpso updated this revision to Diff 64166.Jul 15 2016, 10:56 AM
mssimpso retitled this revision from to [LV] Move vector induction update to end of latch.
mssimpso updated this object.
mssimpso added a reviewer: mkuper.
mssimpso added subscribers: llvm-commits, mcrosier.
mkuper edited edge metadata.Jul 15 2016, 3:49 PM

I don't really see why this should matter, but I don't have any objections either, and I guess uniformity is good.

lib/Transforms/Vectorize/LoopVectorize.cpp
1913 ↗(On Diff #64166)

Since you explicitly moved the instruction to the latch, the phi should probably also take it from the latch.
(The reason this works as is, is because we only support single-BB loops, so the body is always the latch. That was always true, it's just that now it looks more confusing.)

mssimpso added inline comments.Jul 21 2016, 10:29 AM
lib/Transforms/Vectorize/LoopVectorize.cpp
1913 ↗(On Diff #64166)

Right, thanks for catching this. I'll update the patch.

mssimpso updated this revision to Diff 64921.Jul 21 2016, 10:31 AM
mssimpso edited edge metadata.

Addressed Michael's comments.

mkuper accepted this revision.Jul 21 2016, 10:54 AM
mkuper edited edge metadata.

LGTM

This revision is now accepted and ready to land.Jul 21 2016, 10:54 AM
This revision was automatically updated to reflect the committed changes.