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

[Daniel Berlin]

Once done, LGTM

On Mon, May 18, 2015 at 5:35 PM, Daniel Berlin <dberlin at dberlin.org> wrote:
> ================
> Comment at: include/llvm/Transforms/Utils/Local.h:290
> @@ -288,1 +289,3 @@
>
> +/// Replace each use of 'From' with 'To' if that use is dominated by the given
> +/// edge.  Returns the number of replacements made.
> ----------------
> \brief?
>
> ================
> 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)
>
>
> ================
> Comment at: lib/Transforms/Scalar/EarlyCSE.cpp:487
> @@ +486,3 @@
> +                                     BasicBlockEdge(Pred, BB));
> +            // TODO: Exploit icmp eq %v, C to get value for C
> +            // TODO: Look through and/or sequences to get more known conditions
> ----------------
> I'm not sure i agree with either of these TODO's.
> It's really 100% unclear to me how far this rabbithole we should go for EarlyCSE
> I would just leave them (and the above one) out for now, and we can evaluate it when someone has data these are worthwhile to chase.
>
> ================
> Comment at: test/Transforms/EarlyCSE/conditional.ll:1
> @@ +1,2 @@
> +; RUN: opt -early-cse -S < %s | FileCheck %s
> +
> ----------------
> Can you add descriptions of what each test is testing?
> I can kind of figure it out, but not entirely.
>
> Can you also add a few fallthrough edge tests similar to edge.ll from transforms/GVN to make sure it does the right thing?
>
> In fact, i would just copy that and shove the right answers in for earlycse's current impl.
>
> That will make sure nobody accidentally breaks the trickier cases here.
>
> http://reviews.llvm.org/D9763
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
>