[PATCH] D130039: [InstCombine] Improve folding of mul + icmp

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexander-shaposhnikov added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:2032
+    }
+    if (Pred == ICmpInst::ICMP_SLT || Pred == ICmpInst::ICMP_SGE)
+      NewC = ConstantInt::get(
----------------
spatel wrote:
> With icmp predicate canonicalization, I don't think we need to handle SGE/SLE/ULE/UGE. You could assert that those predicates are not present at this point.
Adding an assert makes bootstrapped Clang crash (with the newly added assertion failure).
The crash doesn't happen immediately but on a few inputs, so it looks like they are not fully canonicalized at the moment. 
What would you say  to adding a TODO/FIXME here (to remove  SGE/SLE/ULE/UGE + add assert) ?
(we can file a separate issue)



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130039