Page MenuHomePhabricator

[LV] Avoid emitting trivially dead instructions
ClosedPublic

Authored by mssimpso on Oct 14 2016, 12:14 PM.

Details

Summary

Some instructions from the original loop, when vectorized, can become trivially dead. This happens because of the way we structure the new loop. For example, we create new induction variables and induction variable "steps" in the new loop. Thus, when we go to vectorize the original induction variable update, it may no longer be needed due to the instructions we've already created. This patch prevents us from creating these redundant instructions. This reduces code size before simplification and allows greater flexibility in code generation since we have fewer unnecessary instruction uses.

Diff Detail

Repository
rL LLVM

Event Timeline

mssimpso updated this revision to Diff 74727.Oct 14 2016, 12:14 PM
mssimpso retitled this revision from to [LV] Avoid emitting trivially dead instructions.
mssimpso updated this object.
mssimpso added reviewers: anemet, mkuper, gilr.
mssimpso added subscribers: llvm-commits, mcrosier.
mkuper edited edge metadata.Oct 14 2016, 1:36 PM

I assume the motivation is that we don't want to run adce after the vectorizer, and "regular" dce doesn't catch this because of loop-carried dependencies?

I assume the motivation is that we don't want to run adce after the vectorizer, and "regular" dce doesn't catch this because of loop-carried dependencies?

No, this is mostly motivated by the sinking patch I submitted. These trivially dead instructions can create uses of scalar instructions that can potentially be sunk into predicated blocks. They prevent sinking because if we were to move the scalars, their definitions would no longer dominate the uses, even though the uses should later be deleted.

gilr added inline comments.Oct 17 2016, 6:56 AM
lib/Transforms/Vectorize/LoopVectorize.cpp
4241 ↗(On Diff #74727)

Perhaps replace the double-negation with "... if all its users except the induction variable are dead"?

4246 ↗(On Diff #74727)

The "|| U == Cmp" test seems to kill the update whether or not Cmp is dead. Per the above comment I think you meant here "|| DeadInstructions.count(U)", right?

mssimpso added inline comments.Oct 17 2016, 7:19 AM
lib/Transforms/Vectorize/LoopVectorize.cpp
4241 ↗(On Diff #74727)

That's much better. Thanks!

4246 ↗(On Diff #74727)

Yes, nice catch! I'll update the patch.

mssimpso updated this revision to Diff 75018.Oct 18 2016, 9:07 AM
mssimpso edited edge metadata.

Addressed Gil's comments. Thanks!

mssimpso marked 2 inline comments as done.Oct 18 2016, 9:08 AM
mkuper accepted this revision.Oct 18 2016, 11:05 AM
mkuper edited edge metadata.

LGTM with a nit about naming.

lib/Transforms/Vectorize/LoopVectorize.cpp
4243 ↗(On Diff #75018)

Ind/Induction combined with the two "autos" is confusing. Maybe make the type of Ind explicit (or call it IndPhi?)?

This revision is now accepted and ready to land.Oct 18 2016, 11:05 AM
mssimpso added inline comments.Oct 18 2016, 11:15 AM
lib/Transforms/Vectorize/LoopVectorize.cpp
4243 ↗(On Diff #75018)

I'll make the type explicit. Thanks Michael!

gilr accepted this revision.Oct 18 2016, 12:00 PM
gilr edited edge metadata.

LGTM

This revision was automatically updated to reflect the committed changes.
mssimpso marked an inline comment as done.