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

[David Majnemer via llvm-commits]

majnemer accepted this revision.
majnemer added a reviewer: majnemer.

================
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));
+      }
+    }
   }
----------------
majnemer wrote:
> 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?
On second thought, this is a nice canonicalization regardless because it makes the comparisons easier to analyze.  LGTM.


http://reviews.llvm.org/D18841