[PATCH] D61622: [FastISel][X86] If selectFNeg fails, fall back to SelectionDAG not treating it as an fsub.

Mon May 6 17:59:02 PDT 2019

craig.topper marked an inline comment as done.
craig.topper added inline comments.

================
Comment at: llvm/test/CodeGen/X86/fast-isel-fneg.ll:68
 ; SSE2-NEXT:    movsd {{.*#+}} xmm0 = mem[0],zero
-; SSE2-NEXT:    subsd (%ecx), %xmm0
+; SSE2-NEXT:    xorps {{\.LCPI.*}}, %xmm0
 ; SSE2-NEXT:    movsd %xmm0, (%eax)
----------------
cameron.mcinally wrote:
> The general idea is good, but I'm failing to see how this instruction changed from subsd to xorps. Am I missing something subtle?
The subsd was being emited by selectBinary after selectFNeg returned false. With this change we don't go into selectBinary there anymore for fsub -0.0, x. Instead we return false from selectOperator when selectFNeg fails. This causes fast instruction selection to abort and we'll instead generate a SelectionDAG for the fneg and everything that comes before it. Then we go through normal DAG combines and operation legalization on the SelectionDAG. This causes the LowerFNEGOrFABS code in X86ISelLowering.cpp to execute. This will generate a vector xor. This bad for compile time since we SelectionDAG is slower than just emitting the fsub, but its more correct for this case.

I think once we fail out of SelectOperator we may have another chance to handle this via the target specific fastSelectInstruction hook. We might be able to generate the xorps code manually from there and avoid the SelectionDAG fallback. But I wanted to get a correct implementation first before an optimal one.

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

https://reviews.llvm.org/D61622