[PATCH] Select Elimination in InstCombine

Eric Christopher echristo at gmail.com
Tue Sep 9 15:05:27 PDT 2014


Agreed. I don't mind making it required, but it'd then be nice to use it in
more than this one case.

-eric

On Tue, Sep 9, 2014 at 2:55 PM, Juergen Ributzka <juergen at apple.com> wrote:

> 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> 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
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140909/14e3225b/attachment.html>


More information about the llvm-commits mailing list