[PATCH] D90924: [ConstantRange] Sign-flipping of signedness-invariant comparisons

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/IR/ConstantRange.cpp:159
+
+bool ConstantRange::areInsensitiveToSignednessOfSwappedICmpPredicate(
+    const ConstantRange &CR1, const ConstantRange &CR2) {
----------------
Personally I'd convert this to `areInsensitiveToSignednessOfInvertedICmpPredicate()`, in which case the implementation would be the direct conjugate of the above, i.e.

```
  if (CR1.isEmptySet() || CR2.isEmptySet())
    return true;
        
  return (CR1.isAllNonNegative() && CR2.isAllNegative()) ||
         (CR1.isAllNegative() && CR2.isAllNonNegative());
```

I believe the only difference would be in the treatment of ranges that have a single element intersection (in which case the right canonicalization is probably towards an equality comparison rather than a different sign).


================
Comment at: llvm/unittests/IR/ConstantRangeTest.cpp:2535
+  for (auto Pred : seq((unsigned)ICmpInst::Predicate::FIRST_ICMP_PREDICATE,
+                       ICmpInst::Predicate::LAST_ICMP_PREDICATE + 1)) {
+    if (ICmpInst::isEquality((ICmpInst::Predicate)Pred))
----------------
nikic wrote:
> Use `seq_inclusive()` to avoid the back and forth casts?
Isn't it possible to drop the `(ICmpInst::Predicate)` casts if seq_inclusive is used?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90924