[PATCH] D76319: [DAGCombiner] Fix non-determinism problem related to argument evaluation order in visitFDIV

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope updated this revision to Diff 250926.
bjope added a comment.

Fix formatting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76319

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


Index: llvm/test/CodeGen/X86/fdiv.ll
===================================================================
--- llvm/test/CodeGen/X86/fdiv.ll
+++ llvm/test/CodeGen/X86/fdiv.ll
@@ -76,5 +76,28 @@
   ret <4 x float> %div
 }
 
+; This used to crash, depending on how llc was built (e.g. using clang/gcc),
+; due to order of argument evaluation not being well defined. We ended up
+; hitting llvm_unreachable in getNegatedExpression when building with gcc.
+define float @fdiv_fneg_combine(float %a0, float %a1, float %a2) #0 {
+; CHECK-LABEL: fdiv_fneg_combine:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    movaps %xmm0, %xmm3
+; CHECK-NEXT:    subss %xmm1, %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:    retq
+  %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 %mul2, %sub4
+  ret float %div5
+}
+
 attributes #0 = { "unsafe-fp-math"="false" }
 
Index: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -13043,11 +13043,18 @@
   }
 
   // (fdiv (fneg X), (fneg Y)) -> (fdiv X, Y)
-  if (isCheaperToUseNegatedFPOps(N0, N1))
-    return DAG.getNode(
-        ISD::FDIV, SDLoc(N), VT,
-        TLI.getNegatedExpression(N0, DAG, LegalOperations, ForCodeSize),
-        TLI.getNegatedExpression(N1, DAG, LegalOperations, ForCodeSize), Flags);
+  if (isCheaperToUseNegatedFPOps(N0, N1)) {
+    // Notice that the order of these getNegatedExpression calls seem to
+    // matter. There is a test case in test/CodeGen/X86/fdiv.ll that crashed
+    // when doing the calls in reversed order (e.g. by letting the order depend
+    // on argument evaluation order by folding the calls inside the getNode()
+    // call).
+    SDValue Neg0 =
+        TLI.getNegatedExpression(N0, DAG, LegalOperations, ForCodeSize);
+    SDValue Neg1 =
+        TLI.getNegatedExpression(N1, DAG, LegalOperations, ForCodeSize);
+    return DAG.getNode(ISD::FDIV, SDLoc(N), VT, Neg0, Neg1, Flags);
+  }
 
   return SDValue();
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D76319.250926.patch
Type: text/x-patch
Size: 2375 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200317/44ddfe3b/attachment.bin>