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

[Hans Wennborg via llvm-commits]

hans marked 5 inline comments as done.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:2749
@@ +2748,3 @@
+
+  if (TLI.getOperationAction(ISD::SETCC_PARTS, LHSLo.getValueType()) ==
+      TargetLowering::Custom) {
----------------
DavidKreitzer wrote:
> This looks reasonable to me.
> 
> I am just wondering whether it would be better to refactor this such that we always generate SETCC_PARTS here regardless of whether the target has a custom lowering and then implement a default lowering that does most of what this routine is currently doing.
> 
Can you provide an example of how always generating SETCC_PARTS and having a default lowering would help? One problem I see is that we'd need to legalize it too.

For example a 128-bit SETCC could be legalized to 64-bit SETCC_PARTS, and on a 32-bit target we'd need to legalize that too. With the current patch, we only legalize SETCC to SETCC_PARTS for types the target has said it can efficiently lower.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:14584
@@ +14583,3 @@
+  // other relations, we need to flip the operands.
+  SDValue LHSHi = Op.getOperand(0);
+  SDValue RHSHi = Op.getOperand(1);
----------------
DavidKreitzer wrote:
> Your operand ordering for SETCC_PARTS doesn't match your comment in ISDOpcodes.h. The two should at least be in sync, and I already gave my opinion on what the operand order ought to be. :-)
Oops. Thanks for catching this. So much for trying to document things :-) I've updated the code to use the order you suggested.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:14614
@@ +14613,3 @@
+
+  SDVTList VTs = DAG.getVTList(LHSHi.getValueType(), MVT::i32);
+  SDValue LowSub = DAG.getNode(X86ISD::SUB, DL, VTs, LHSLo, RHSLo);
----------------
DavidKreitzer wrote:
> The code sequences you are generating here look great!
Thanks to you and Michael for suggesting it :-)

================
Comment at: test/CodeGen/X86/wide-integer-cmp.ll:79
@@ +78,3 @@
+; CHECK: movl 12(%esp), [[RHSLo:%[a-z]+]]
+; CHECK: movl 16(%esp), [[LHSHi:%[a-z]+]]
+; CHECK: cmpl 4(%esp), [[RHSLo]]
----------------
DavidKreitzer wrote:
> Also RHSHi here and at line 81.
> 
Thanks! Not sure how I managed to get them so backwards.


http://reviews.llvm.org/D14496