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

[Dávid Bolvanský via Phabricator via llvm-commits]

xbolva00 added inline comments.


================
Comment at: llvm/lib/Analysis/BranchProbabilityInfo.cpp:885
     // Otherwise -> Likely
     if (CI->isTrueWhenEqual())
       std::swap(TakenProb, UntakenProb);
----------------
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.


================
Comment at: llvm/lib/Analysis/BranchProbabilityInfo.cpp:980
+    // X != C -> Likely
+    switch (CI->getPredicate()) {
+    case CmpInst::ICMP_EQ:
----------------
ebrevnov wrote:
> Looks like we want to set "likely" for equal and "unlikely" not equal regardless of actual value of a constant, right? In that case I would suggest to just check that case before processing specific constant values. Moreover recently added code at line 882 should be checking exactly the same condition for not constant values, right?
I got perf regression when I used same condition from line 882.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86017