[PATCH] D30477: [SCEV] Compute affine range in another way to avoid bitwidth extending.

[Michael Zolotukhin via Phabricator via llvm-commits]

mzolotukhin marked 7 inline comments as done.
mzolotukhin added a comment.

Hi,

Thanks for the initial review, please find the patch updated.

Michael



================
Comment at: lib/Analysis/ScalarEvolution.cpp:4683
+// C <  A <= B
+static bool tripleUCompare(APInt A, APInt B, APInt C) {
+  if (A.ule(B) && B.ult(C))
----------------
sanjoy wrote:
> `tripleUCompare` is not too descriptive -- what is it actually //doing//?  Rather, what property of `A`, `B`, `C` is it testing for?
I'm using `ConstantRange::contains` instead. I'd really appreciate more eyes here to check if I'm not missing anything.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:4716
+  // By how much the expression can change.
+  APInt Offset = Step * MaxBECount;
+
----------------
sanjoy wrote:
> Is it okay for this to overflow?
Fixed.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:4762
+static ConstantRange getSignedRangeForAR(APInt Step, ConstantRange StartRange,
+                                         APInt MaxBECount, bool BackDirection,
+                                         unsigned BitWidth) {
----------------
sanjoy wrote:
> Do you need to pass in `BackDirection` here?  Can't you get the same information by checking the sign of `Step`?
Fixed.


https://reviews.llvm.org/D30477