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

[Joshua Cao via Phabricator via llvm-commits]

caojoshua accepted this revision.
caojoshua added inline comments.
This revision is now accepted and ready to land.


================
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));
+}
----------------
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.



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

https://reviews.llvm.org/D148661