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

[Evgeniy via Phabricator via llvm-commits]

ebrevnov added inline comments.


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


================
Comment at: llvm/lib/Analysis/BranchProbabilityInfo.cpp:980
+    // X != C -> Likely
+    switch (CI->getPredicate()) {
+    case CmpInst::ICMP_EQ:
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86017