[PATCH] D148661: [SCEV] Common code for computing trip count in a fixed type [NFC-ish]

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:8051
+  // Get the total trip count from the count by adding 1.
+  return getAddExpr(getZeroExtendExpr(ExitCount, EvalTy), getOne(EvalTy));
+}
----------------
caojoshua wrote:
> Does this break existing calls to `getTripCountFromExitCount()`? Now all trip counts will be ZeroExtended.
> 
> Although this is not breaking any current tests, I do have https://reviews.llvm.org/D141823 open which depends on removing the zext when possible when computing trip multiples.
It does not.  We know this because returning the wider type was the interface of this function before your recent change.  

As for your second point, relying on a narrower type being returned (instead of doing the inference) is highly suspicious.  You can rebase here and propose a follow on if needed.  


================
Comment at: llvm/lib/Transforms/Scalar/LoopFlatten.cpp:319
+  // Evaluating in the trip counts type can not overflow here as the overflow
+  // checks are performed in checkOverflow, but are first tried to avoid by
+  // widening the IV.
----------------
craig.topper wrote:
> "avoid by widening"
In the original text.  I didn't want to reword as I wasn't 100% sure I understood the intent.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148661