[PATCH] D87832: [IndVars] Remove monotonic checks with unknown exit count

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

IOW is this missing some precondition (potentially just a comment) that in signed case, the trip count is a positive number?



================
Comment at: llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:2375-2388
+  // The predicate must be monotonic.
+  switch (Pred) {
+  default:
+    return false;
+  case ICmpInst::ICMP_SLT:
+  case ICmpInst::ICMP_ULT:
+  case ICmpInst::ICMP_SGT:
----------------
It might be good to hoist that into some ICmp method,
this is at least the second such code block.


================
Comment at: llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:2412-2422
+  // Because step is +/- 1 and MaxIter has same type as Start (i.e. it does
+  // not exceed max unsigned value of this type), this effectively proves
+  // that there is no wrap during the iteration. To prove that there is no
+  // signed/unsigned wrap, we need to check that
+  // Start <= Last for step = 1 or Start >= Last for step = -1.
+  ICmpInst::Predicate NoOverflowPred =
+      CmpInst::isSigned(Pred) ? ICmpInst::ICMP_SLE : ICmpInst::ICMP_ULE;
----------------
I agree about `unsigned` case:
```
----------------------------------------
define i1 @src(i32 %base, i32 %num) {
%0:
  %t0 = uadd_overflow {i32, i1, i24} %base, %num
  %t1 = extractvalue {i32, i1, i24} %t0, 1
  %t2 = xor i1 %t1, 1
  ret i1 %t2
}
=>
define i1 @tgt(i32 %base, i32 %num) {
%0:
  %t0 = add i32 %base, %num
  %t1 = icmp ule i32 %base, %t0
  ret i1 %t1
}
Transformation seems to be correct!
```
but `signed` case seems to be different:
```
----------------------------------------
define i1 @src(i32 %base, i32 %num) {
%0:
  %t0 = sadd_overflow {i32, i1, i24} %base, %num
  %t1 = extractvalue {i32, i1, i24} %t0, 1
  %t2 = xor i1 %t1, 1
  ret i1 %t2
}
=>
define i1 @tgt(i32 %base, i32 %num) {
%0:
  %t0 = add i32 %base, %num
  %t1 = icmp sle i32 %base, %t0
  ret i1 %t1
}
Transformation doesn't verify!
ERROR: Value mismatch

Example:
i32 %base = #x34600ff0 (878710768)
i32 %num = #xcba01020 (3416264736, -878702560)

Source:
{i32, i1, i24} %t0 = { #x00002010 (8208), #x0 (0), poison }
i1 %t1 = #x0 (0)
i1 %t2 = #x1 (1)

Target:
i32 %t0 = #x00002010 (8208)
i1 %t1 = #x0 (0)
Source value: #x1 (1)
Target value: #x0 (0)
```
```
----------------------------------------
define i1 @src(i32 %base, i31 %num_) {
%0:
  %num = zext i31 %num_ to i32
  %t0 = sadd_overflow {i32, i1, i24} %base, %num
  %t1 = extractvalue {i32, i1, i24} %t0, 1
  %t2 = xor i1 %t1, 1
  ret i1 %t2
}
=>
define i1 @tgt(i32 %base, i31 %num_) {
%0:
  %num = zext i31 %num_ to i32
  %t0 = add i32 %base, %num
  %t1 = icmp sle i32 %base, %t0
  ret i1 %t1
}
Transformation seems to be correct!

```


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

https://reviews.llvm.org/D87832