[PATCH] D32143: [InstSimplify] use ConstantRange to simplify more and-of-icmps

Sanjoy Das via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 20 15:37:24 PDT 2017


sanjoy requested changes to this revision.
sanjoy added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/Analysis/InstructionSimplify.cpp:1592
       return getFalse(ITy);
+
+    // If a range is a superset of the other, the smaller set is all we need.
----------------
As written, the code is correct but misleading.

I think the correct thing to do in this case is:

```
    auto Range0 = ConstantRange::makeExactICmpRegion(Pred0, *C0);
    auto Range1 = ConstantRange::makeExactICmpRegion(Pred1, *C1);
```

The current code is correct because for an `APInt` (as opposed to `ConstantRange`) RHS, `makeExactICmpRegion` just happens to be the same as `makeAllowedICmpRegion`.  However, if `*C0` and `*C1` were `ConstantRange` s (i.e. someone generalized this code), then using `makeAllowedICmpRegion` would be wrong.



https://reviews.llvm.org/D32143





More information about the llvm-commits mailing list