[PATCH] D76439: [SDAG] fix crash in getNegatedExpression() from altered number of uses

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 19 11:27:22 PDT 2020


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

We've discussed this problem before in D70975 <https://reviews.llvm.org/D70975>. Copying directly from that description:
We check the number of uses as a predicate for whether some value is free to negate, but that use count can change as we rewrite the expression in getNegatedExpression(). So something that was marked free to negate during the cost evaluation phase becomes not free to negate during the rewrite phase (or the inverse - something that was not free becomes free). This can lead to a crash/assert because we expect that everything in an expression that is negatible to be handled in the corresponding code within getNegatedExpression().

The problem was similarly partly fixed in D75501 <https://reviews.llvm.org/D75501>, but we missed that the same behavior could originate from fmul/fdiv.

This is another incomplete fix for the whole problem, but I think that requires integrating getNegatibleCost() and getNegatedExpression(), so that we can't alter costs between analysis and rewrite.


https://reviews.llvm.org/D76439

Files:
  llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
  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:    subss %xmm2, %xmm1
+; CHECK-NEXT:    mulss %xmm0, %xmm1
+; CHECK-NEXT:    subss %xmm0, %xmm3
+; CHECK-NEXT:    divss %xmm1, %xmm3
+; CHECK-NEXT:    movss %xmm3, (%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/lib/CodeGen/SelectionDAG/TargetLowering.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -5761,9 +5761,13 @@
   case ISD::FMUL:
   case ISD::FDIV: {
     SDValue X = Op.getOperand(0), Y = Op.getOperand(1);
-    // fold (fneg (fmul X, Y)) -> (fmul (fneg X), Y)
     NegatibleCost CostX = getNegatibleCost(X, DAG, LegalOps, OptForSize, Depth);
-    if (CostX != NegatibleCost::Expensive)
+    NegatibleCost CostY = getNegatibleCost(Y, DAG, LegalOps, OptForSize, Depth);
+    // TODO: The comparison is inverted from what we might expect of a "cost"
+    //       because the enum values have "Cheaper" with a larger numerical
+    //       value than "Expensive".
+    if (CostX >= CostY)
+      // fold (fneg (fmul X, Y)) -> (fmul (fneg X), Y)
       return DAG.getNode(
           Opcode, DL, VT,
           getNegatedExpression(X, DAG, LegalOps, OptForSize, Depth), Y, Flags);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D76439.251422.patch
Type: text/x-patch
Size: 2357 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200319/5629c545/attachment.bin>


More information about the llvm-commits mailing list