[PATCH] D55414: [X86] Emit SBB instead of SETCC_CARRY from LowerSELECT. Break false dependency on the SBB input.

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added a comment.

Hi Craig,

Overall the patch looks good for Intel processors. For AMD  (at least Jaguar and other modern AMD processors), an SBB with same register operands is a dependency breaking instruction. To be more specific, an SBB with same reg operands depends on the CF only. So we don’t need a zero idiom XOR to explicitly break the data dependency on the input register operands. That means, the old codegen was better for AMD.

I don’t know what is the best approach in this case.
Maybe you can add a FIXME comment and raise a bug for it, so that we can address it later. It would allow you to commit this change in the meantime. Not sure if @RKSimon has a better idea.



================
Comment at: test/CodeGen/X86/select.ll:790
 ; CHECK:       ## %bb.0:
+; CHECK-NEXT:    xorl %eax, %eax
 ; CHECK-NEXT:    cmpq $1, %rdi
----------------
It is a shame that SBB with same registers is not treated specially by Intel hips.
On Jaguar (and other modern AMD processors), an SBB where register operands are the same is a dependency-breaking instruction.
That means, this XOR is redundant on Jaguar, and there is no need for it to break a dependency on EAX.


================
Comment at: test/CodeGen/X86/vector-compare-any_of.ll:55
+; AVX-NEXT:    xorl %eax, %eax
+; AVX-NEXT:    cmpl %ecx, %eax
 ; AVX-NEXT:    sbbq %rax, %rax
----------------
Same “problem”. The original code was probably slightly better for Jaguar.


================
Comment at: test/CodeGen/X86/vector-compare-any_of.ll:90
+; SSE-NEXT:    sbbl %ecx, %ecx
+; SSE-NEXT:    movslq %ecx, %rax
 ; SSE-NEXT:    retq
----------------
same.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55414/new/

https://reviews.llvm.org/D55414