[PATCH] D19261: X86 _comi_ intrinsics - Fixed lowering

[David Kreitzer via llvm-commits]

DavidKreitzer added a comment.

This approach seems fine for now, since it is an improvement over what currently exists. But it seems to me that we would like to unify the handling of the comi intrinsics with the treatment of the FP comparisons in general, at least to the extent possible.


================
Comment at: ../lib/Target/X86/X86ISelLowering.cpp:17596
@@ +17595,3 @@
+
+      SDValue Comi = DAG.getNode(IntrData->Opc0, dl, MVT::i32, LHS, RHS);
+      SDValue SetCC1 = DAG.getNode(X86ISD::SETCC, dl, MVT::i8,
----------------
This expansion is overkill for the comparison conditions that can be implemented with a single SETcc. For example, a GT comparison can be implemented with a simple SETA. And an LT comparison can be implemented by reversing operands and using SETA. Only EQ and NEQ need to be implemented with 2 SETcc instructions.

Also, this expansion is incorrect for NEQ. NEQ needs to be implemented as SETNE || SETP.

Fixing these 2 problems will affect the changes you made to the tests, of course.


================
Comment at: ../test/CodeGen/X86/avx-intrinsics-x86.ll:1
@@ -1,1 +1,2 @@
+; NOTE: Assertions have been autogenerated by update_llc_test_checks.py
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
----------------
Did you intend to add this line (in light of a nearly identical line 2)?

================
Comment at: ../test/CodeGen/X86/avx512-intrinsics.ll:1
@@ -1,1 +1,2 @@
+; NOTE: Assertions have been autogenerated by update_llc_test_checks.py
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
----------------
Again, did you intend to add this line?

================
Comment at: ../test/CodeGen/X86/avx512-intrinsics.ll:6850
@@ -6857,4 +6849,3 @@
 ; CHECK-NEXT:    kmovw %edi, %k1
-; CHECK-NEXT:    vpmovzxbd %xmm0, %zmm1 {%k1}
-; CHECK-NEXT:    vpmovzxbd %xmm0, %zmm2 {%k1} {z}
-; CHECK-NEXT:    vpmovzxbd %xmm0, %zmm0
+; CHECK-NEXT:    vpmovzxbd {{.*#+}} zmm1 = xmm0[0],zero,zero,zero,xmm0[1],zero,zero,zero,xmm0[2],zero,zero,zero,xmm0[3],zero,zero,zero,xmm0[4],zero,zero,zero,xmm0[5],zero,zero,zero,xmm0[6],zero,zero,zero,xmm0[7],zero,zero,zero,xmm0[8],zero,zero,zero,xmm0[9],zero,zero,zero,xmm0[10],zero,zero,zero,xmm0[11],zero,zero,zero,xmm0[12],zero,zero,zero,xmm0[13],zero,zero,zero,xmm0[14],zero,zero,zero,xmm0[15],zero,zero,zero
+; CHECK-NEXT:    vpmovzxbd {{.*#+}} zmm2 = xmm0[0],zero,zero,zero,xmm0[1],zero,zero,zero,xmm0[2],zero,zero,zero,xmm0[3],zero,zero,zero,xmm0[4],zero,zero,zero,xmm0[5],zero,zero,zero,xmm0[6],zero,zero,zero,xmm0[7],zero,zero,zero,xmm0[8],zero,zero,zero,xmm0[9],zero,zero,zero,xmm0[10],zero,zero,zero,xmm0[11],zero,zero,zero,xmm0[12],zero,zero,zero,xmm0[13],zero,zero,zero,xmm0[14],zero,zero,zero,xmm0[15],zero,zero,zero
----------------
Did you intend to include all these vpmovzx changes in this patch? They look unrelated.



Repository:
  rL LLVM

http://reviews.llvm.org/D19261