[PATCH] fix an invisible bug when combining repeated FP divisors

[Sanjay Patel]

Adding bogus inline replies in an attempt to get the real replies that I sent yesterday to the commits list.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:8312
@@ -8311,3 +8311,3 @@
       SDValue FPOne = DAG.getConstantFP(1.0, DL, VT);
       SDValue Reciprocal = DAG.getNode(ISD::FDIV, DL, VT, FPOne, N1);
 
----------------
spatel wrote:
> hfinkel wrote:
> > So once we have FMF, you'll add them here?
> Right - once we have FMF, this recip node should at least have 'arcp'. And that's a bug in the existing patch in D9708 ( http://reviews.llvm.org/D9708 ) because I forgot about that.
> 
> And I think any FMUL that we create below would also have FMF because it was created under an FMF precondition.
.

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:8324
@@ -8321,1 +8323,3 @@
+          // same node as Reciprocal, but with FMF they may be different nodes.
+          CombineTo(U, Reciprocal);
         }
----------------
spatel wrote:
> hfinkel wrote:
> > I'm not sure this is the right solution. Instead, we may want to check for a pre-existing reciprocal node, and use it if it already exists instead of replacing it with, perhaps, looser FMFs.
> > 
> This is where things get even foggier for me. :)
> To make this optimization, don't we need to check the FMF of each U node? Eg, if one of these users does not have 'arcp', it is not eligible for replacement. I did check this in the current draft in D9708.
.

http://reviews.llvm.org/D9893

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/