[PATCH] D14496: X86: More efficient codegen for 64-bit compares on 32-bit target

[David Kreitzer via llvm-commits]

DavidKreitzer added a comment.

I really like the way this turned out!  Thanks so much for doing the rework. I just had a couple minor additional suggestions.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:2772
@@ +2771,3 @@
+
+  // Lower with SETCCE if the target supports it.
+  if (TLI.getOperationAction(
----------------
I would expect every target to be able to support this (at which point, much of the code in this routine can be removed). Maybe it would be a good idea to add a FIXME comment to that effect.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:14595
@@ +14594,3 @@
+  switch (cast<CondCodeSDNode>(Cond)->get()) {
+  // This operation can only detect < and >=.
+  case ISD::SETLT:  CC = X86::COND_L; break;
----------------
I would recommend against this restriction. The SETCCE operation itself makes perfect sense for other conditions. (For example, SETCCE "eq" computes op0 - op1 - carry == 0.) For the purposes of large integer compare lowering, it happens to only be useful for < and >=, but we might find other good uses for it. (I'm imagining some fancy DAG combine optimizations ...)


================
Comment at: test/CodeGen/X86/wide-integer-cmp.ll:86
@@ +85,3 @@
+
+define i32 @test_wide(i128 %a, i128 %b) {
+entry:
----------------
Nice! This is SO much better than the current code!


http://reviews.llvm.org/D14496