[llvm-commits] Fold multiply by 0 and 1 when in UnsafeFPMath mode in SelectionDAG::getNode()

[Chandler Carruth]

On Fri, Sep 7, 2012 at 5:24 PM, Owen Anderson <resistor at mac.com> wrote:

> Chandler,
>
> We can't write a test case to exercise specifically this path, because it
> is not an observable change on any in-tree targets, since the DAG combiner
> also performs the same transform.  Observing the difference between the two
> requires specific interactions with target-specific DAG combine's.  The
> best we could do would be to write a test that will exercise whichever path
> happens to get run, but it will *not* break if this code gets broken.
>
> That said, it's still worthwhile from a compile time perspective to
> aggressively prevent the creation of new SDNode's (with all their
> associated baggage) rather than leaving it up to the DAG combiner.  You'll
> notice that the code this patch touches already does the same thing for
> FADD and FSUB.
>

Oh sure, it just wasn't clear that the dag combiner already had code for
this from the initial email. =] Maybe include a note about that in the
commit log.

<insert pining over the lack of a unittest framework for the selection DAG
that would allow us to write such a test>

Tiny, minor patch comment that doesn't really matter. LGTM in general.

+      } else if (Opcode == ISD::FMUL) {
+        if (ConstantFPSDNode *CFP = dyn_cast<ConstantFPSDNode>(N1)) {
+          // 0*x --> 0
+          if (CFP->isZero())
+            return N1;
+          // 1*x --> x
+          if (CFP->isExactlyValue(1.0))
+            return N2;
+        }
+        if (ConstantFPSDNode *CFP = dyn_cast<ConstantFPSDNode>(N2)) {

Rather than duplicating the logic you could write:

ConstantFPSDNode *CFP = dyn_cast<ConstantFPSDNode>(N1)
if (!CFP)
  CFP = dyn_cast<ConstantFPSDNode>(N2)
if (CFP) {
  // transform logic
}
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120907/d411cefa/attachment.html>