[PATCH] D45598: [DAGCombiner, PowerPC] allow X - (fpext(-Y) --> X + fpext(Y) with multiple uses

[Sanjay Patel via Phabricator via llvm-commits]

spatel created this revision.
spatel added reviewers: escha, nemanjai, hfinkel, efriedma.
Herald added a subscriber: mcrosier.

This is a transform that I limited in instcombine in https://reviews.llvm.org/rL329821 because it was creating more instructions in IR when the cast has multiple uses.

However, in the post-commit mailing list thread for that change, @escha wrote that an out-of-tree target had regressed from that change because fpext is free on that target. Since I don't have access to that target, I'm using PowerPC as a stand-in to show the benefit of the transform here in the backend. PowerPC is the only in-tree target that sets isFPExtFree() to 'true'.

If this looks right, then we may need to add another hook for isFPTruncFree() to fully recover from the IR change because it affected fptrunc casts too.


https://reviews.llvm.org/D45598

Files:
  lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  test/CodeGen/PowerPC/fsub-fneg.ll


Index: test/CodeGen/PowerPC/fsub-fneg.ll
===================================================================
--- test/CodeGen/PowerPC/fsub-fneg.ll
+++ test/CodeGen/PowerPC/fsub-fneg.ll
@@ -1,16 +1,16 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le < %s | FileCheck %s
 
-; FIXME: When fpext is free, we should look through it for optimizations
+; When fpext is free, we should look through it for optimizations
 ; even if it has multiple uses and produce an 'fadd' here.
 ; Y - (fpext(-X)) --> Y + fpext(X)
 
 define double @neg_ext_op1_extra_use(float %x, double %y) nounwind {
 ; CHECK-LABEL: neg_ext_op1_extra_use:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    fneg 0, 1
-; CHECK-NEXT:    xssubdp 1, 2, 0
-; CHECK-NEXT:    xsdivdp 1, 0, 1
+; CHECK-NEXT:    xsadddp 0, 2, 1
+; CHECK-NEXT:    fneg 13, 1
+; CHECK-NEXT:    xsdivdp 1, 13, 0
 ; CHECK-NEXT:    blr
   %t1 = fsub float -0.0, %x
   %t2 = fpext float %t1 to double
Index: lib/CodeGen/SelectionDAG/DAGCombiner.cpp
===================================================================
--- lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -667,13 +667,16 @@
   // fneg is removable even if it has multiple uses.
   if (Op.getOpcode() == ISD::FNEG) return 2;
 
-  // Don't allow anything with multiple uses.
-  if (!Op.hasOneUse()) return 0;
+  // Don't allow anything with multiple uses unless we know it is free.
+  EVT VT = Op.getValueType();
+  if (!Op.hasOneUse())
+    if (!(Op.getOpcode() == ISD::FP_EXTEND &&
+          TLI.isFPExtFree(VT, Op.getOperand(0).getValueType())))
+      return 0;
 
   // Don't recurse exponentially.
   if (Depth > 6) return 0;
 
-  EVT VT = Op.getValueType();
   switch (Op.getOpcode()) {
   default: return false;
   case ISD::ConstantFP: {
@@ -736,9 +739,6 @@
   // fneg is removable even if it has multiple uses.
   if (Op.getOpcode() == ISD::FNEG) return Op.getOperand(0);
 
-  // Don't allow anything with multiple uses.
-  assert(Op.hasOneUse() && "Unknown reuse!");
-
   assert(Depth <= 6 && "GetNegatedExpression doesn't match isNegatibleForFree");
 
   const SDNodeFlags Flags = Op.getNode()->getFlags();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D45598.142278.patch
Type: text/x-patch
Size: 2233 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180412/cb4dd230/attachment.bin>