[PATCH] Extend EarlyCSE to handle basic cases from JumpThreading and CVP

Daniel Berlin dberlin at dberlin.org
Tue May 19 08:50:43 PDT 2015


> ================
> Comment at: lib/Transforms/Scalar/EarlyCSE.cpp:471
> @@ +470,3 @@
> +  if (BasicBlock *Pred = BB->getSinglePredecessor())
> +    // TODO: handle switch with case value as well
> +    if (auto *BI = dyn_cast<BranchInst>(Pred->getTerminator()))
> ----------------
> Even GVN does not handle switch with case value, even when it can determine the case value as a constant. It is not as simple as one would thing because it's not about dominance, but control dependence.
>
>
> (and DT->dominates will fail the single edge assertion in those cases, even though it's a reasonable thing to test)
>
>


Just to explain this:

getSinglePredecessor returns true if all edges lead to the same block.
Thus, a switch with multiple edges to the same block will return a
block to getSinglePredecessor.

Dominates(Edge, anything) asserts isSingleEdge(Edge).
isSingleEdge asserts that the edge is singular in the sense that
"multiple edges do not exist from block EdgeStart to block EdgeEnd".
Thus, getSinglePredecessor will happily give you blocks from switch
statements that fail this test (any switch with case statements
leading to the same block), such that you can't test dominance on
them.

GVN goes to the trouble of tracking how many edges lead to a target
block, and only propagates equalities if a single edge reaches a given
target block to avoid this assert.

This is true *even if* it could prove that only one of the edges to
the target block is reachable (because it would still hit the
isSingleEdge assert).

At some point, that assert should simply be removed (or such was the
conclusion of the folks who added it when i asked them, since the
justification given for it makes no sense), but it does mean that
"handling switch with case value" is not a trivial TODO :)

(note, of course, that splitting all critical edges also solves this
particular problem)



More information about the llvm-commits mailing list