[PATCH] Select Elimination in InstCombine

[Gerolf Hoflehner]

Ok, makes sense. The DT is optional and I added the missing check. When it is absent we just don’t recognize as many patterns.

I expect  DT usage to evolve over time, but have no specifics plans.

-Gerolf

On Sep 9, 2014, at 12:52 AM, Eric Christopher <echristo at gmail.com> wrote:

> Hi Gerolf,
> 
> One more comment in line.
> 
> As far as the dominance tree, you don't ever check to find out if you have dominance trees, you just require it. The leading question was: if it's not optional, then you can probably use it in more places in the pass. If it is optional then you're not actually checking for it before using it.
> 
> Thanks!
> 
> ================
> Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2977
> @@ +2976,3 @@
> +              // replace select with operand on then path for NE compares.
> +              int Path = I.getPredicate() == ICmpInst::ICMP_EQ ? 1 : 0;
> +              if (InstCombiner::dominatesAllUses(
> ----------------
> majnemer wrote:
>> echristo wrote:
>>> Isn't this just a boolean then?
>> I'd make this an unsigned, getSuccessor is asking which successor to take.  It just so happens that we are only interested in cases where there are two successors.
> Yes. Makes sense that way. Might be better off just grabbing the correct successor rather than mucking with the integer.
I agree.
> 
> http://reviews.llvm.org/D5258
> 
>