[GVN] Rewrite IsValueFullyAvailableInBlock(): no recursion, less false-negatives
authorRoman Lebedev <lebedev.ri@gmail.com>
Tue, 28 Jul 2020 07:16:52 +0000 (10:16 +0300)
committerRoman Lebedev <lebedev.ri@gmail.com>
Tue, 28 Jul 2020 07:19:28 +0000 (10:19 +0300)
commite40315d2b4ed1e38962a8f33ff151693ed4ada63
tree11f7797e90464b76f467ac9881a9c1c433c654eb
parent486d2750c7151d3d93b785a4669e2d7d5c9286ac
[GVN] Rewrite IsValueFullyAvailableInBlock(): no recursion, less false-negatives

While this doesn't appear to help with the perf issue being exposed by
D84108, the function as-is is very weird, convoluted, and what's worse,
recursive.

There was no need for `SpeculativelyAvaliableAndUsedForSpeculation`,
tri-state choice is enough. We don't even ever check for that state.

The basic idea here is that we need to perform a depth-first traversal
of the predecessors of the basic block in question, either finding a
preexisting state for the block in a map, or inserting a "placeholder"
`SpeculativelyAvaliable`,

If we encounter an `Unavaliable` block, then we need to give up search,
and back-propagate the `Unavaliable` state to the each successor of
said block, more specifically to the each `SpeculativelyAvaliable`
we've just created.

However, if we have traversed entirety of the predecessors and have not
encountered an `Unavaliable` block, then it must mean the value is fully
available. We could update each inserted `SpeculativelyAvaliable` into
a `Avaliable`, but we don't need to, as assertion excersizes,
because we can assume that if we see an `SpeculativelyAvaliable` entry,
it is actually `Avaliable`, because during the time we've produced it,
if we would have found that it has an `Unavaliable` predecessor,
we would have updated it's successors, including this block,
into `Unavaliable`

Reviewed By: fhahn

Differential Revision: https://reviews.llvm.org/D84181
llvm/lib/Transforms/Scalar/GVN.cpp
llvm/test/Transforms/GVN/loadpre-missed-opportunity.ll