[PATCH] D76638: [SDAG] fix crash in getNegatedExpression() by ignoring transient fadd

[Sanjay Patel via Phabricator via llvm-commits]

spatel created this revision.
spatel added reviewers: steven.zhang, RKSimon, bjope, dstuttard.
Herald added subscribers: hiraditya, mcrosier.

This is an alternative proposal to D76439 <https://reviews.llvm.org/D76439> to fix the crashing test. This is similar to a bailout that already exists for "fmul X, 2.0".

An fadd with fneg operand should always be reduced to fsub, so if we encounter that pattern while running getNegatibleCost(), just ignore it until we have a chance to run combines on that fadd node.

In the x86 tests, this produces slightly better code - we form an expression with an fadd rather than all fsub. It does not make a real difference on the examples here, but we prefer the commutability of fadd because that gives register allocation more flexibility to handle destructive SSE ops without extra register moves.


https://reviews.llvm.org/D76638

Files:
  llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
  llvm/test/CodeGen/X86/fdiv.ll
  llvm/test/CodeGen/X86/neg_fp.ll


Index: llvm/test/CodeGen/X86/neg_fp.ll
===================================================================
--- llvm/test/CodeGen/X86/neg_fp.ll
+++ llvm/test/CodeGen/X86/neg_fp.ll
@@ -54,3 +54,32 @@
   %t18 = fadd double %t16, %t7
   ret double %t18
 }
+
+; This would crash because the negated expression for %sub4
+; creates a new use of %sub1 and that alters the negated cost
+
+define float @fdiv_extra_use_changes_cost(float %a0, float %a1, float %a2) nounwind {
+; CHECK-LABEL: fdiv_extra_use_changes_cost:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    pushl %eax
+; CHECK-NEXT:    movss {{.*#+}} xmm0 = mem[0],zero,zero,zero
+; CHECK-NEXT:    movss {{.*#+}} xmm1 = mem[0],zero,zero,zero
+; CHECK-NEXT:    movss {{.*#+}} xmm2 = mem[0],zero,zero,zero
+; CHECK-NEXT:    movaps %xmm2, %xmm3
+; CHECK-NEXT:    subss %xmm1, %xmm3
+; CHECK-NEXT:    mulss %xmm0, %xmm3
+; CHECK-NEXT:    subss %xmm2, %xmm1
+; CHECK-NEXT:    addss %xmm0, %xmm1
+; CHECK-NEXT:    divss %xmm3, %xmm1
+; CHECK-NEXT:    movss %xmm1, (%esp)
+; CHECK-NEXT:    flds (%esp)
+; CHECK-NEXT:    popl %eax
+; CHECK-NEXT:    retl
+  %sub1 = fsub fast float %a0, %a1
+  %mul2 = fmul fast float %sub1, %a2
+  %neg = fneg fast float %a0
+  %add3 = fadd fast float %a1, %neg
+  %sub4 = fadd fast float %add3, %a2
+  %div5 = fdiv fast float %sub4, %mul2
+  ret float %div5
+}
Index: llvm/test/CodeGen/X86/fdiv.ll
===================================================================
--- llvm/test/CodeGen/X86/fdiv.ll
+++ llvm/test/CodeGen/X86/fdiv.ll
@@ -85,11 +85,11 @@
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    movaps %xmm0, %xmm3
 ; CHECK-NEXT:    subss %xmm1, %xmm3
+; CHECK-NEXT:    mulss %xmm2, %xmm3
 ; CHECK-NEXT:    subss %xmm0, %xmm1
-; CHECK-NEXT:    mulss %xmm2, %xmm1
-; CHECK-NEXT:    subss %xmm2, %xmm3
-; CHECK-NEXT:    divss %xmm3, %xmm1
-; CHECK-NEXT:    movaps %xmm1, %xmm0
+; CHECK-NEXT:    addss %xmm2, %xmm1
+; CHECK-NEXT:    divss %xmm1, %xmm3
+; CHECK-NEXT:    movaps %xmm3, %xmm0
 ; CHECK-NEXT:    retq
   %sub1 = fsub fast float %a0, %a1
   %mul2 = fmul fast float %sub1, %a2
Index: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -5629,6 +5629,11 @@
     if (LegalOperations && !isOperationLegalOrCustom(ISD::FSUB, VT))
       return NegatibleCost::Expensive;
 
+    // Ignore fadd with fneg because that will be canonicalized to fsub.
+    if (Op.getOperand(0).getOpcode() == ISD::FNEG ||
+        Op.getOperand(1).getOpcode() == ISD::FNEG)
+      return NegatibleCost::Expensive;
+
     // fold (fneg (fadd A, B)) -> (fsub (fneg A), B)
     NegatibleCost V0 = getNegatibleCost(Op.getOperand(0), DAG, LegalOperations,
                                         ForCodeSize, Depth + 1);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D76638.252124.patch
Type: text/x-patch
Size: 2848 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200323/e0f060c4/attachment.bin>