[PATCH] D70975: [SDAG] remove use restriction in isNegatibleForFree() when called from getNegatedExpression()

[Sanjay Patel via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG36b1232ec5f3: [SDAG] remove use restriction in isNegatibleForFree() when called from… (authored by spatel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70975

Files:
  llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
  llvm/test/CodeGen/AArch64/arm64-fmadd.ll
  llvm/test/CodeGen/X86/fma-fneg-combine-2.ll


Index: llvm/test/CodeGen/X86/fma-fneg-combine-2.ll
===================================================================
--- llvm/test/CodeGen/X86/fma-fneg-combine-2.ll
+++ llvm/test/CodeGen/X86/fma-fneg-combine-2.ll
@@ -45,15 +45,15 @@
 define float @test_fneg_fma_subx_suby_negz_f32(float %w, float %x, float %y, float %z)  {
 ; FMA3-LABEL: test_fneg_fma_subx_suby_negz_f32:
 ; FMA3:       # %bb.0: # %entry
-; FMA3-NEXT:    vsubss %xmm0, %xmm1, %xmm1
-; FMA3-NEXT:    vsubss %xmm2, %xmm0, %xmm0
+; FMA3-NEXT:    vsubss %xmm1, %xmm0, %xmm1
+; FMA3-NEXT:    vsubss %xmm0, %xmm2, %xmm0
 ; FMA3-NEXT:    vfmadd213ss {{.*#+}} xmm0 = (xmm1 * xmm0) + xmm3
 ; FMA3-NEXT:    retq
 ;
 ; FMA4-LABEL: test_fneg_fma_subx_suby_negz_f32:
 ; FMA4:       # %bb.0: # %entry
-; FMA4-NEXT:    vsubss %xmm0, %xmm1, %xmm1
-; FMA4-NEXT:    vsubss %xmm2, %xmm0, %xmm0
+; FMA4-NEXT:    vsubss %xmm1, %xmm0, %xmm1
+; FMA4-NEXT:    vsubss %xmm0, %xmm2, %xmm0
 ; FMA4-NEXT:    vfmaddss %xmm3, %xmm0, %xmm1, %xmm0
 ; FMA4-NEXT:    retq
 entry:
@@ -86,4 +86,24 @@
   ret float %1
 }
 
+; This would crash while trying getNegatedExpression().
+
+define float @negated_constant(float %x) {
+; FMA3-LABEL: negated_constant:
+; FMA3:       # %bb.0:
+; FMA3-NEXT:    vmulss {{.*}}(%rip), %xmm0, %xmm1
+; FMA3-NEXT:    vfmadd132ss {{.*#+}} xmm0 = (xmm0 * mem) + xmm1
+; FMA3-NEXT:    retq
+;
+; FMA4-LABEL: negated_constant:
+; FMA4:       # %bb.0:
+; FMA4-NEXT:    vmulss {{.*}}(%rip), %xmm0, %xmm1
+; FMA4-NEXT:    vfmaddss %xmm1, {{.*}}(%rip), %xmm0, %xmm0
+; FMA4-NEXT:    retq
+  %m = fmul float %x, 42.0
+  %fma = call nsz float @llvm.fma.f32(float %x, float -42.0, float %m)
+  %nfma = fneg float %fma
+  ret float %nfma
+}
+
 declare float @llvm.fma.f32(float, float, float)
Index: llvm/test/CodeGen/AArch64/arm64-fmadd.ll
===================================================================
--- llvm/test/CodeGen/AArch64/arm64-fmadd.ll
+++ llvm/test/CodeGen/AArch64/arm64-fmadd.ll
@@ -88,5 +88,23 @@
   ret double %0
 }
 
+; This would crash while trying getNegatedExpression().
+
+define float @negated_constant(float %x) {
+; CHECK-LABEL: negated_constant:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    mov w8, #-1037565952
+; CHECK-NEXT:    mov w9, #1109917696
+; CHECK-NEXT:    fmov s1, w8
+; CHECK-NEXT:    fmul s1, s0, s1
+; CHECK-NEXT:    fmov s2, w9
+; CHECK-NEXT:    fmadd s0, s0, s2, s1
+; CHECK-NEXT:    ret
+  %m = fmul float %x, 42.0
+  %fma = call nsz float @llvm.fma.f32(float %x, float -42.0, float %m)
+  %nfma = fneg float %fma
+  ret float %nfma
+}
+
 declare float @llvm.fma.f32(float, float, float) nounwind readnone
 declare double @llvm.fma.f64(double, double, double) nounwind readnone
Index: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -5631,7 +5631,14 @@
                                  ForCodeSize, Depth + 1);
     char V1 = isNegatibleForFree(Op.getOperand(1), DAG, LegalOperations,
                                  ForCodeSize, Depth + 1);
-    if (V0 >= V1) {
+    // TODO: This is a hack. It is possible that costs have changed between now
+    //       and the initial calls to isNegatibleForFree(). That is because we
+    //       are rewriting the expression, and that may change the number of
+    //       uses (and therefore the cost) of values. If the negation costs are
+    //       equal, only negate this value if it is a constant. Otherwise, try
+    //       operand 1. A better fix would eliminate uses as a cost factor or
+    //       track the change in uses as we rewrite the expression.
+    if (V0 > V1 || (V0 == V1 && isa<ConstantFPSDNode>(Op.getOperand(0)))) {
       // fold (fneg (fma X, Y, Z)) -> (fma (fneg X), Y, (fneg Z))
       SDValue Neg0 = getNegatedExpression(
           Op.getOperand(0), DAG, LegalOperations, ForCodeSize, Depth + 1);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D70975.234343.patch
Type: text/x-patch
Size: 3959 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191217/2f24adc0/attachment.bin>