[PATCH] Select Elimination in InstCombine

[Hal Finkel]

----- Original Message -----
> From: "Gerolf Hoflehner" <ghoflehner at apple.com>
> To: reviews+D5258+public+f9996d3e9899d3c8 at reviews.llvm.org
> Cc: llvm-commits at cs.uiuc.edu
> Sent: Tuesday, September 9, 2014 4:50:52 PM
> Subject: Re: [PATCH] Select Elimination in InstCombine
> 
> 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.

FWIW, when you rebase this you'll notice that I've already added an optional DT to InstCombine (as part of the @llvm.assume infrastructure). So it is now already there.

 -Hal

> 
> 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
> 

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