[PATCH] Select Elimination in InstCombine

[Hal Finkel]

----- Original Message -----
> From: "Eric Christopher" <echristo at gmail.com>
> To: "Juergen Ributzka" <juergen at apple.com>
> Cc: "LLVM Commits" <llvm-commits at cs.uiuc.edu>, reviews+D5258+public+f9996d3e9899d3c8 at reviews.llvm.org
> Sent: Tuesday, September 9, 2014 5:05:27 PM
> Subject: Re: [PATCH] Select Elimination in InstCombine
> 
> 
> 
> Agreed. I don't mind making it required, but it'd then be nice to use
> it in more than this one case.

FWIW, it will also be used by all of the simplification logic that can make use of @llvm.assume (which is actually a large amount of it because it essentially comes down to anything that calls into ValueTracking).

 -Hal

> 
> 
> -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
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory