[all-commits] [llvm/llvm-project] e40315: [GVN] Rewrite IsValueFullyAvailableInBlock(): no r...

Roman Lebedev via All-commits all-commits at lists.llvm.org
Tue Jul 28 00:27:53 PDT 2020


  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: e40315d2b4ed1e38962a8f33ff151693ed4ada63
      https://github.com/llvm/llvm-project/commit/e40315d2b4ed1e38962a8f33ff151693ed4ada63
  Author: Roman Lebedev <lebedev.ri at gmail.com>
  Date:   2020-07-28 (Tue, 28 Jul 2020)

  Changed paths:
    M llvm/lib/Transforms/Scalar/GVN.cpp
    M llvm/test/Transforms/GVN/loadpre-missed-opportunity.ll

  Log Message:
  -----------
  [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




More information about the All-commits mailing list