[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:
> reames wrote:
> > 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.  
> > As for your second point, relying on a narrower type being returned (instead of doing the inference) is highly suspicious. 
> 
> I think its fair to simplify a SCEV as much as possible so that inference does not have to do as much work. The change does not rely on a narrower type, but it does rely on the `+1` getting folded into the zext. It is explained in https://reviews.llvm.org/D147117.
> 
I finally figured out the confusion here.  My landed patch was adjusted to use the other routine, and should address your concern here.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148661