[PATCH] D34278: [InstCombine] Recognize and simplify three way comparison idioms

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2454
+                     m_ConstantInt(Less), m_ConstantInt(Greater)))) {
+    if (PredA == ICmpInst::ICMP_EQ && PredB == ICmpInst::ICMP_SLT)
+      return true;
----------------
We can check PredA right after we matched line 2450 to avoid further maching if we have a wrong comparison there.


================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2454
+                     m_ConstantInt(Less), m_ConstantInt(Greater)))) {
+    if (PredA == ICmpInst::ICMP_EQ && PredB == ICmpInst::ICMP_SLT)
+      return true;
----------------
mkazantsev wrote:
> We can check PredA right after we matched line 2450 to avoid further maching if we have a wrong comparison there.
Maybe just another && instead of second if?


================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2474
+                                 C1LessThan, C2Equal, C3GreaterThan)) {
+    assert(C1LessThan && C2Equal && C3GreaterThan && C);
+
----------------
Can we assert C on method entry?


================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2495
+    Value *Cond = Builder->getFalse();
+    if (TrueWhenLessThan)
+      Cond = Builder->CreateOr(Cond, Builder->CreateICmp(ICmpInst::ICMP_SLT, OrigLHS, OrigRHS));
----------------
Is it possible that more than one of {TrueWhenLessThan, TrueWhenEqual, TrueWhenGreaterThan} is true?

If no, please add an assertion on that and use "else if" below.
If yes, should we not consider this when creating conditions?


https://reviews.llvm.org/D34278