[PATCH] D34532: [LoopUnroll] Fix bug in computeUnrollCount causing it to not honor MaxCount

[Anna Thomas via Phabricator via llvm-commits]

anna added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:813
             (LoopSize - UP.BEInsns);
       if (UP.Count > UP.MaxCount)
         UP.Count = UP.MaxCount;
----------------
This check should actually be present along all paths from this function?


================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:839
     }
+    if (UP.Count > UP.MaxCount)
+      UP.Count = UP.MaxCount;
----------------
What happens if we early return from this function with result as `true`, but UP.Count > UP.MaxCount?

Perhaps we should be setting the max value at the caller of this function :
```
tryToUnrollLoop() {
....
 // computeUnrollCount() decides whether it is beneficial to use upper bound to fully 
 // unroll the loop.
  bool UseUpperBound = false;
  bool IsCountSetExplicitly =
      computeUnrollCount(L, TTI, DT, LI, SE, &ORE, TripCount, MaxTripCount,
                         TripMultiple, LoopSize, UP, UseUpperBound);
  if (!UP.Count)
    return false;
  // Unroll factor (Count) must be less or equal to TripCount.
  if (TripCount && UP.Count > TripCount)
    UP.Count = TripCount;
  // Add the check after this?

}
```


https://reviews.llvm.org/D34532