[x86] convert logic-of-FP-compares to FP logic-of-vector-compares

[spatel]

Summary

This is motivated by the examples and discussion in:
https://llvm.org/PR51245
...and related bugs.

By using vector compares and vector logic, we can convert 2 'set' instructions into 1 'movd' or 'movmsk' and generally improve throughput/reduce instructions.

Unfortunately, we don't have a complete vector compare ISA before AVX, so I left SSE-only out of this patch. Ie, we'd need extra logic ops to simulate the missing predicates for SSE 'cmpp*', so it's not as clearly a win.

[RKSimon]

llvm/lib/Target/X86/X86ISelLowering.cpp

45511

Why not use SCALAR_TO_VECTOR ?

llvm/test/CodeGen/X86/fcmp-logic.ll

155

These could still be merged (as long as we only use the lower 32-bits).?

[RKSimon]

llvm/test/CodeGen/X86/fcmp-logic.ll

3

We should probably add AVX512 coverage to ensure we're not doing anything that could be done better with kmask registers

[spatel]

llvm/lib/Target/X86/X86ISelLowering.cpp

45511

No reason - forgot we had that opcode.

llvm/test/CodeGen/X86/fcmp-logic.ll

155

Yes - we'd need to add a cast to make it work, but hopefully that would get folded away at some point.
I'll add a TODO for now.

[spatel]

Patch updated:

1. Use SCALAR_TO_VECTOR opcode to reduce code.
2. Added avx512f to test to show k-reg codegen.
3. Added TODO comment about enhancement for mismatched FP types.

[RKSimon]

LGTM - cheers

I don't think this patch will cover it, but as a follow up please could you add test coverage for logic containing more than 2 fcmps?

llvm/lib/Target/X86/X86ISelLowering.cpp

45510

Do we need to check/assert that VT is MVT::i1 ?

[pengfei]

llvm/lib/Target/X86/X86ISelLowering.cpp

45453–45454

Nit, why use hyphen here but leave space above?

45510

I think it's Ok since the logic operands come from setcc directly. Do we need to handle (logic (zext (setcc ...?

[pengfei]

llvm/test/CodeGen/X86/fcmp-logic.ll

157–174

Should we add a common CHECK for them?

[spatel]

llvm/lib/Target/X86/X86ISelLowering.cpp

45453–45454

I just overlooked the first one - will fix it.

45510

I haven't been able to find an example where we have the pattern late only, but the result of x86 scalar setcc could be i8 after some transforms.

There's no difference on the existing tests if I add the check for i1, so I'll do that.

Also, the pattern where we have sext/zext between the logic op and setcc is already folded away by DAGCombiner (the cast is moved after the logic), so I don't think we need to worry about that case.

llvm/test/CodeGen/X86/fcmp-logic.ll

157–174

It's the "v" in the mnemonics that makes these different. Do we have a scrubber option in the script for that?

[spatel]

Patch updated:

1. Made "floating-point" consistent in the function comment.
2. Added MVT::i1 constraint for safety.

[pengfei]

LGTM.

llvm/test/CodeGen/X86/fcmp-logic.ll

157–174

Oh, yes. I don't think there's such an option. I did have the same thought. But I think it's not easy to achieve. It's valuable only when we check SSE and AVX instructions at the same time. Omitting 'v' unconditionally is over kill.

llvm/lib/Target/X86/X86ISelLowering.cpp

static unsigned convertIntLogicToFPLogicOpcode(unsigned Opcode) {
   case ISD::AND: FPOpcode = X86ISD::FAND; break;
   case ISD::OR:  FPOpcode = X86ISD::FOR;  break;
   case ISD::XOR: FPOpcode = X86ISD::FXOR; break;
   }
   return FPOpcode;
 }
 
 /// If both input operands of a logic op are being cast from floating point
-/// types, try to convert this into a floating point logic node to avoid
-/// unnecessary moves from SSE to integer registers.
+/// types or FP compares, try to convert this into a floating-point logic node
+/// to avoid unnecessary moves from SSE to integer registers.
 static SDValue convertIntLogicToFPLogic(SDNode *N, SelectionDAG &DAG,
                                         TargetLowering::DAGCombinerInfo &DCI,
                                         const X86Subtarget &Subtarget) {
   EVT VT = N->getValueType(0);
   SDValue N0 = N->getOperand(0);
   SDValue N1 = N->getOperand(1);
   SDLoc DL(N);
 
-  if (N0.getOpcode() != ISD::BITCAST || N1.getOpcode() != ISD::BITCAST)
-    return SDValue();
-
-  if (DCI.isBeforeLegalizeOps())
+  if (!((N0.getOpcode() == ISD::BITCAST && N1.getOpcode() == ISD::BITCAST) ||
+        (N0.getOpcode() == ISD::SETCC && N1.getOpcode() == ISD::SETCC)))
     return SDValue();
 
   SDValue N00 = N0.getOperand(0);
   SDValue N10 = N1.getOperand(0);
   EVT N00Type = N00.getValueType();
   EVT N10Type = N10.getValueType();
 
   // Ensure that both types are the same and are legal scalar fp types.
   if (N00Type != N10Type || !((Subtarget.hasSSE1() && N00Type == MVT::f32) ||
                               (Subtarget.hasSSE2() && N00Type == MVT::f64) ||
                               (Subtarget.hasFP16() && N00Type == MVT::f16)))
     return SDValue();
 
+  if (N0.getOpcode() == ISD::BITCAST && !DCI.isBeforeLegalizeOps()) {
     unsigned FPOpcode = convertIntLogicToFPLogicOpcode(N->getOpcode());
     SDValue FPLogic = DAG.getNode(FPOpcode, DL, N00Type, N00, N10);
     return DAG.getBitcast(VT, FPLogic);
   }
 
+  // The vector ISA for FP predicates is incomplete before AVX, so converting
+  // COMIS* to CMPS* may not be a win before AVX.
+  // TODO: Check types/predicates to see if they are available with SSE/SSE2.
+  if (N0.getOpcode() != ISD::SETCC || !N0.hasOneUse() || !N1.hasOneUse() ||
+      !Subtarget.hasAVX())
+    return SDValue();
+
+  // Convert scalar FP compares and logic to vector compares (COMIS* to CMPS*)
+  // and vector logic:
+  // logic (setcc N00, N01), (setcc N10, N11) -->
+  // setcc0 = (setcc (inselt undef, N00, 0), (inselt undef, N01, 0))
+  // setcc1 = (setcc (inselt undef, N10, 0), (inselt undef, N11, 0))
+  // extelt (logic setcc0, setcc1), 0
+  unsigned NumElts = 128 / N00Type.getSizeInBits();
+  EVT VecVT = EVT::getVectorVT(*DAG.getContext(), N00Type, NumElts);
+  EVT BoolVecVT = EVT::getVectorVT(*DAG.getContext(), MVT::i1, NumElts);
+  SDValue BaseVec = DAG.getUNDEF(VecVT);
+  SDValue ZeroIndex = DAG.getVectorIdxConstant(0, DL);
+  SDValue N01 = N0.getOperand(1);
+  SDValue N11 = N1.getOperand(1);
+  SDValue InsN00 =
+      DAG.getNode(ISD::INSERT_VECTOR_ELT, DL, VecVT, BaseVec, N00, ZeroIndex);
+  SDValue InsN01 =
+      DAG.getNode(ISD::INSERT_VECTOR_ELT, DL, VecVT, BaseVec, N01, ZeroIndex);
+  SDValue InsN10 =
+      DAG.getNode(ISD::INSERT_VECTOR_ELT, DL, VecVT, BaseVec, N10, ZeroIndex);
+  SDValue InsN11 =
+      DAG.getNode(ISD::INSERT_VECTOR_ELT, DL, VecVT, BaseVec, N11, ZeroIndex);
+  SDValue Setcc0 = DAG.getSetCC(DL, BoolVecVT, InsN00, InsN01,
+                                cast<CondCodeSDNode>(N0.getOperand(2))->get());
+  SDValue Setcc1 = DAG.getSetCC(DL, BoolVecVT, InsN10, InsN11,
+                                cast<CondCodeSDNode>(N1.getOperand(2))->get());
+  SDValue Logic = DAG.getNode(N->getOpcode(), DL, BoolVecVT, Setcc0, Setcc1);
+  return DAG.getNode(ISD::EXTRACT_VECTOR_ELT, DL, VT, Logic, ZeroIndex);
+}
+
 // Attempt to fold BITOP(MOVMSK(X),MOVMSK(Y)) -> MOVMSK(BITOP(X,Y))
 // to reduce XMM->GPR traffic.
 static SDValue combineBitOpWithMOVMSK(SDNode *N, SelectionDAG &DAG) {
   unsigned Opc = N->getOpcode();
   assert((Opc == ISD::OR || Opc == ISD::AND || Opc == ISD::XOR) &&
          "Unexpected bit opcode");
 
   SDValue N0 = N->getOperand(0);

llvm/test/CodeGen/X86/fcmp-logic.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=x86_64-- -mattr=sse2 | FileCheck %s --check-prefixes=SSE
 ; RUN: llc < %s -mtriple=x86_64-- -mattr=avx  | FileCheck %s --check-prefixes=AVX
 
 define i1 @olt_ole_and_f32(float %w, float %x, float %y, float %z) {
 ; SSE-LABEL: olt_ole_and_f32:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    ucomiss %xmm0, %xmm1
 ; SSE-NEXT:    seta %cl
 ; SSE-NEXT:    ucomiss %xmm2, %xmm3
 ; SSE-NEXT:    setae %al
 ; SSE-NEXT:    andb %cl, %al
 ; SSE-NEXT:    retq
 ;
 ; AVX-LABEL: olt_ole_and_f32:
 ; AVX:       # %bb.0:
-; AVX-NEXT:    vucomiss %xmm0, %xmm1
-; AVX-NEXT:    seta %cl
-; AVX-NEXT:    vucomiss %xmm2, %xmm3
-; AVX-NEXT:    setae %al
-; AVX-NEXT:    andb %cl, %al
+; AVX-NEXT:    vcmpleps %xmm3, %xmm2, %xmm2
+; AVX-NEXT:    vcmpltps %xmm1, %xmm0, %xmm0
+; AVX-NEXT:    vandps %xmm2, %xmm0, %xmm0
+; AVX-NEXT:    vmovd %xmm0, %eax
+; AVX-NEXT:    # kill: def $al killed $al killed $eax
 ; AVX-NEXT:    retq
   %f1 = fcmp olt float %w, %x
   %f2 = fcmp ole float %y, %z
   %r = and i1 %f1, %f2
   ret i1 %r
 }
 
 define i1 @oge_oeq_or_f32(float %w, float %x, float %y, float %z) {
 ; SSE-LABEL: oge_oeq_or_f32:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    ucomiss %xmm1, %xmm0
 ; SSE-NEXT:    setae %cl
 ; SSE-NEXT:    ucomiss %xmm3, %xmm2
 ; SSE-NEXT:    setnp %dl
 ; SSE-NEXT:    sete %al
 ; SSE-NEXT:    andb %dl, %al
 ; SSE-NEXT:    orb %cl, %al
 ; SSE-NEXT:    retq
 ;
 ; AVX-LABEL: oge_oeq_or_f32:
 ; AVX:       # %bb.0:
-; AVX-NEXT:    vucomiss %xmm1, %xmm0
-; AVX-NEXT:    setae %cl
-; AVX-NEXT:    vucomiss %xmm3, %xmm2
-; AVX-NEXT:    setnp %dl
-; AVX-NEXT:    sete %al
-; AVX-NEXT:    andb %dl, %al
-; AVX-NEXT:    orb %cl, %al
+; AVX-NEXT:    vcmpeqps %xmm3, %xmm2, %xmm2
+; AVX-NEXT:    vcmpleps %xmm0, %xmm1, %xmm0
+; AVX-NEXT:    vorps %xmm2, %xmm0, %xmm0
+; AVX-NEXT:    vmovd %xmm0, %eax
+; AVX-NEXT:    # kill: def $al killed $al killed $eax
 ; AVX-NEXT:    retq
   %f1 = fcmp oge float %w, %x
   %f2 = fcmp oeq float %y, %z
   %r = or i1 %f1, %f2
   ret i1 %r
 }
 
 define i1 @ord_one_xor_f32(float %w, float %x, float %y, float %z) {
 ; SSE-LABEL: ord_one_xor_f32:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    ucomiss %xmm1, %xmm0
 ; SSE-NEXT:    setnp %cl
 ; SSE-NEXT:    ucomiss %xmm3, %xmm2
 ; SSE-NEXT:    setne %al
 ; SSE-NEXT:    xorb %cl, %al
 ; SSE-NEXT:    retq
 ;
 ; AVX-LABEL: ord_one_xor_f32:
 ; AVX:       # %bb.0:
-; AVX-NEXT:    vucomiss %xmm1, %xmm0
-; AVX-NEXT:    setnp %cl
-; AVX-NEXT:    vucomiss %xmm3, %xmm2
-; AVX-NEXT:    setne %al
-; AVX-NEXT:    xorb %cl, %al
+; AVX-NEXT:    vcmpneq_oqps %xmm3, %xmm2, %xmm2
+; AVX-NEXT:    vcmpordps %xmm1, %xmm0, %xmm0
+; AVX-NEXT:    vxorps %xmm2, %xmm0, %xmm0
+; AVX-NEXT:    vmovd %xmm0, %eax
+; AVX-NEXT:    # kill: def $al killed $al killed $eax
 ; AVX-NEXT:    retq
   %f1 = fcmp ord float %w, %x
   %f2 = fcmp one float %y, %z
   %r = xor i1 %f1, %f2
   ret i1 %r
 }
 
 define i1 @une_ugt_and_f64(double %w, double %x, double %y, double %z) {
 ; SSE-LABEL: une_ugt_and_f64:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    ucomisd %xmm1, %xmm0
 ; SSE-NEXT:    setp %al
 ; SSE-NEXT:    setne %cl
 ; SSE-NEXT:    orb %al, %cl
 ; SSE-NEXT:    ucomisd %xmm2, %xmm3
 ; SSE-NEXT:    setb %al
 ; SSE-NEXT:    andb %cl, %al
 ; SSE-NEXT:    retq
 ;
 ; AVX-LABEL: une_ugt_and_f64:
 ; AVX:       # %bb.0:
-; AVX-NEXT:    vucomisd %xmm1, %xmm0
-; AVX-NEXT:    setp %al
-; AVX-NEXT:    setne %cl
-; AVX-NEXT:    orb %al, %cl
-; AVX-NEXT:    vucomisd %xmm2, %xmm3
-; AVX-NEXT:    setb %al
-; AVX-NEXT:    andb %cl, %al
+; AVX-NEXT:    vcmpnlepd %xmm3, %xmm2, %xmm2
+; AVX-NEXT:    vcmpneqpd %xmm1, %xmm0, %xmm0
+; AVX-NEXT:    vandpd %xmm2, %xmm0, %xmm0
+; AVX-NEXT:    vmovd %xmm0, %eax
+; AVX-NEXT:    # kill: def $al killed $al killed $eax
 ; AVX-NEXT:    retq
   %f1 = fcmp une double %w, %x
   %f2 = fcmp ugt double %y, %z
   %r = and i1 %f1, %f2
   ret i1 %r
 }
 
 define i1 @ult_uge_or_f64(double %w, double %x, double %y, double %z) {
 ; SSE-LABEL: ult_uge_or_f64:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    ucomisd %xmm1, %xmm0
 ; SSE-NEXT:    setb %cl
 ; SSE-NEXT:    ucomisd %xmm2, %xmm3
 ; SSE-NEXT:    setbe %al
 ; SSE-NEXT:    orb %cl, %al
 ; SSE-NEXT:    retq
 ;
 ; AVX-LABEL: ult_uge_or_f64:
 ; AVX:       # %bb.0:
-; AVX-NEXT:    vucomisd %xmm1, %xmm0
-; AVX-NEXT:    setb %cl
-; AVX-NEXT:    vucomisd %xmm2, %xmm3
-; AVX-NEXT:    setbe %al
-; AVX-NEXT:    orb %cl, %al
+; AVX-NEXT:    vcmpnltpd %xmm3, %xmm2, %xmm2
+; AVX-NEXT:    vcmpnlepd %xmm0, %xmm1, %xmm0
+; AVX-NEXT:    vorpd %xmm2, %xmm0, %xmm0
+; AVX-NEXT:    vmovd %xmm0, %eax
+; AVX-NEXT:    # kill: def $al killed $al killed $eax
 ; AVX-NEXT:    retq
   %f1 = fcmp ult double %w, %x
   %f2 = fcmp uge double %y, %z
   %r = or i1 %f1, %f2
   ret i1 %r
 }
 
 define i1 @une_uno_xor_f64(double %w, double %x, double %y, double %z) {
 ; SSE-LABEL: une_uno_xor_f64:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    ucomisd %xmm1, %xmm0
 ; SSE-NEXT:    setp %al
 ; SSE-NEXT:    setne %cl
 ; SSE-NEXT:    orb %al, %cl
 ; SSE-NEXT:    ucomisd %xmm3, %xmm2
 ; SSE-NEXT:    setp %al
 ; SSE-NEXT:    xorb %cl, %al
 ; SSE-NEXT:    retq
 ;
 ; AVX-LABEL: une_uno_xor_f64:
 ; AVX:       # %bb.0:
-; AVX-NEXT:    vucomisd %xmm1, %xmm0
-; AVX-NEXT:    setp %al
-; AVX-NEXT:    setne %cl
-; AVX-NEXT:    orb %al, %cl
-; AVX-NEXT:    vucomisd %xmm3, %xmm2
-; AVX-NEXT:    setp %al
-; AVX-NEXT:    xorb %cl, %al
+; AVX-NEXT:    vcmpunordpd %xmm3, %xmm2, %xmm2
+; AVX-NEXT:    vcmpneqpd %xmm1, %xmm0, %xmm0
+; AVX-NEXT:    vxorpd %xmm2, %xmm0, %xmm0
+; AVX-NEXT:    vmovd %xmm0, %eax
+; AVX-NEXT:    # kill: def $al killed $al killed $eax
 ; AVX-NEXT:    retq
   %f1 = fcmp une double %w, %x
   %f2 = fcmp uno double %y, %z
   %r = xor i1 %f1, %f2
   ret i1 %r
 }
 
+; This uses ucomis because the types do not match.
+
 define i1 @olt_olt_and_f32_f64(float %w, float %x, double %y, double %z) {
 ; SSE-LABEL: olt_olt_and_f32_f64:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    ucomiss %xmm0, %xmm1
 ; SSE-NEXT:    seta %cl
 ; SSE-NEXT:    ucomisd %xmm2, %xmm3
 ; SSE-NEXT:    seta %al
 ; SSE-NEXT:    andb %cl, %al
 ; SSE-NEXT:    retq
 ;
 ; AVX-LABEL: olt_olt_and_f32_f64:
 ; AVX:       # %bb.0:
 ; AVX-NEXT:    vucomiss %xmm0, %xmm1
 ; AVX-NEXT:    seta %cl
 ; AVX-NEXT:    vucomisd %xmm2, %xmm3
 ; AVX-NEXT:    seta %al
 ; AVX-NEXT:    andb %cl, %al
 ; AVX-NEXT:    retq
   %f1 = fcmp olt float %w, %x
   %f2 = fcmp olt double %y, %z
   %r = and i1 %f1, %f2
   ret i1 %r
 }
 
+; This uses ucomis because of extra uses.
+
 define i1 @une_uno_xor_f64_use1(double %w, double %x, double %y, double %z, i1* %p) {
 ; SSE-LABEL: une_uno_xor_f64_use1:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    ucomisd %xmm1, %xmm0
 ; SSE-NEXT:    setp %al
 ; SSE-NEXT:    setne %cl
 ; SSE-NEXT:    orb %al, %cl
 ; SSE-NEXT:    movb %cl, (%rdi)

 ; AVX-NEXT:    retq
   %f1 = fcmp une double %w, %x
   store i1 %f1, i1* %p
   %f2 = fcmp uno double %y, %z
   %r = xor i1 %f1, %f2
   ret i1 %r
 }
 
+; This uses ucomis because of extra uses.
+
 define i1 @une_uno_xor_f64_use2(double %w, double %x, double %y, double %z, i1* %p) {
 ; SSE-LABEL: une_uno_xor_f64_use2:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    ucomisd %xmm1, %xmm0
 ; SSE-NEXT:    setp %al
 ; SSE-NEXT:    setne %cl
 ; SSE-NEXT:    orb %al, %cl
 ; SSE-NEXT:    ucomisd %xmm3, %xmm2

llvm/test/CodeGen/X86/lzcnt-zext-cmp.ll

 }
 
 ; PR31902 Fix a crash in combineOrCmpEqZeroToCtlzSrl under fast math.
 define i32 @test_zext_cmp11(double %a, double %b) "no-nans-fp-math"="true" {
 ;
 ; ALL-LABEL: test_zext_cmp11:
 ; ALL:       # %bb.0: # %entry
 ; ALL-NEXT:    vxorpd %xmm2, %xmm2, %xmm2
-; ALL-NEXT:    vucomisd %xmm2, %xmm0
-; ALL-NEXT:    sete %al
-; ALL-NEXT:    vucomisd %xmm2, %xmm1
-; ALL-NEXT:    sete %cl
-; ALL-NEXT:    orb %al, %cl
-; ALL-NEXT:    movzbl %cl, %eax
+; ALL-NEXT:    vcmpeqpd %xmm2, %xmm1, %xmm1
+; ALL-NEXT:    vcmpeqpd %xmm2, %xmm0, %xmm0
+; ALL-NEXT:    vorpd %xmm1, %xmm0, %xmm0
+; ALL-NEXT:    vmovd %xmm0, %eax
+; ALL-NEXT:    andl $1, %eax
 ; ALL-NEXT:    retq
 entry:
   %cmp = fcmp fast oeq double %a, 0.000000e+00
   %cmp1 = fcmp fast oeq double %b, 0.000000e+00
   %0 = or i1 %cmp, %cmp1
   %conv = zext i1 %0 to i32
   ret i32 %conv
 }