[PATCH] D86017: [BPI] Improve static heuristics for integer comparisons with CST

Dávid Bolvanský via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 17 02:25:03 PDT 2020


xbolva00 added inline comments.


================
Comment at: llvm/lib/Analysis/BranchProbabilityInfo.cpp:885
     // Otherwise -> Likely
     if (CI->isTrueWhenEqual())
       std::swap(TakenProb, UntakenProb);
----------------
ebrevnov wrote:
> xbolva00 wrote:
> > ebrevnov wrote:
> > > xbolva00 wrote:
> > > > ebrevnov wrote:
> > > > > Looks like I misread this code originally. I thought the intent was to set "likely" if equal and "unlikely" if not. But as I understand now isTrueWhenEqual does completely different thing. I would suggest reverting this ASAP and rework.
> > > > All perf inprovement is gone when only == is unlikely. So atleast the comment should be fixed.
> > > It should be not only == is unlikely, but != is likely as well.  Doesn't this help?
> > > What exact condition you have for which you need to set probabilities?
> > > 
> >  if (CI->isTrueWhenEqual())
> > 
> > so in this case
> > X == Y / X >= Y  / X <= Y -> Unlikely
> > 
> > Otherwise likely.
> > 
> > When I used
> >  if (Pred == EQ) then unlikely else likely, I got perf problems. So I think that LT/GT are key predicates which helps us here. 
> >  if (CI->isTrueWhenEqual())
> > 
> > so in this case
> > X == Y / X >= Y  / X <= Y -> Unlikely
> You set unlikely to both X>=Y and X<=Y. That doesn't make any sense to me. Moreover I don't see prerequisites to predict any of these to be more or less likely. If in you case that happen to be the case you probably can come up with an extra analysis to match the pattern. Another thing to consider is balance between complexity of analysis and expected gain.
> 
> > 
> > Otherwise likely.
> > 
> > When I used
> >  if (Pred == EQ) then unlikely else likely, I got perf problems.
> Don't do "else likely" part. What I'm suggesting is:
> if (Pred == EQ) unlikely;
> else if (Pred == NE) likely;
> else unset;
> 
> > So I think that LT/GT are key predicates which helps us here.
> You can't predict which one holds in general. In other words why a>b is more likely than a< b? Is there any specific in your application which can help to favor one but not the other?
> 
> 
> 
Ok, see updated patch.

Decompression speed is now smaller a bit, but compression speed was improved a bit.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86017/new/

https://reviews.llvm.org/D86017



More information about the llvm-commits mailing list