[PATCH] D71990: [LoopUtils] Better accuracy for getLoopEstimatedTripCount.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/LoopUtils.cpp:719
+  uint64_t HeaderBlockWeight = LatchCycleWeight + LatchExitWeight;
+  return llvm::divideNearest(HeaderBlockWeight, LatchExitWeight);
 }
----------------
Would be better to separate the functional (+1) change from the variable name changes and divideNearest(). The latter is an NFC which is independent of and hides the former.

If the functional change goes first, it can e.g. simply do

```
+  uint64_t EstimatedBackedgeTakenCount;
  if (LatchBR->getSuccessor(0) == L->getHeader())
-    return (TrueVal + (FalseVal / 2)) / FalseVal;
+    EstimatedBackedgeTakenCount = (TrueVal + (FalseVal / 2)) / FalseVal;
  else
-    return (FalseVal + (TrueVal / 2)) / TrueVal;
+    EstimatedBackedgeTakenCount = (FalseVal + (TrueVal / 2)) / TrueVal;
+  return EstimatedBackedgeTakenCount + 1;
```
Can alternatively renamed original function to getEstimatedBackedgeTakenCount(), and introduced a new getLoopEstimatedTripCount() which returns the former plus one, if it's useful(?)


================
Comment at: llvm/unittests/Transforms/Utils/CMakeLists.txt:18
   LocalTest.cpp
+  LoopUtilsTest.cpp
   SizeOptsTest.cpp
----------------
already there?


================
Comment at: llvm/unittests/Transforms/Utils/LoopUtilsTest.cpp:1
+//===--------- LoopUtilsTest.cpp - unit tests -----------------------------===//
+//
----------------
"This file was added." - this file already exists, right?
What is this trying to check on top of the above fixed lit tests?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71990