[PATCH] D18841: [InstCombine] Canonicalize icmp instructions based on dominating conditions.

[David Majnemer via llvm-commits]

majnemer added inline comments.

================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:3306-3317
@@ -3273,1 +3305,14 @@
+        return replaceInstUsesWith(I, Builder->getTrue());
+      // Canonicalizing a sign bit comparison that gets used in a branch,
+      // pessimizes codegen by generating branch on zero instruction instead
+      // of a test and branch. So we avoid canonicalizing in such situations
+      // because test and branch instruction has better branch displacement
+      // than compare and branch instruction.
+      if (!isBranchOnSignBitCheck(I, isSignBit) && !I.isEquality()) {
+        if (auto *AI = Intersection.getSingleElement())
+          return new ICmpInst(ICmpInst::ICMP_EQ, Op0, Builder->getInt(*AI));
+        if (auto *AD = Difference.getSingleElement())
+          return new ICmpInst(ICmpInst::ICMP_NE, Op0, Builder->getInt(*AD));
+      }
+    }
   }
----------------
The prior two replacements seem like pure goodness: they make conditional branches branch on true or false.  These two seem weird to have in InstCombine.  Seeing as how you are concerned with pessimization, can we move these to CGP and have them operate on a TargetLowering hook?


http://reviews.llvm.org/D18841