[PATCH] Select Elimination in InstCombine

[Juergen Ributzka]

The problem with making DT optional is that you might never get it. If any pass that runs before InstCombine invalidates DT, it won’t be available to any following pass until there is a pass that explicitly requires it.

-Juergen

> On Sep 9, 2014, at 2:50 PM, Gerolf Hoflehner <ghoflehner at apple.com> wrote:
> 
> 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 <mailto: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 <http://reviews.llvm.org/D5258>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140909/8cf48434/attachment.html>