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

[Michael Ilseman]

Thanks for the comment. New patch attached. I'm not too familiar with SDValue/SDNode, so if you see a way that it can be cleaned up further, let me know!


On Sep 7, 2012, at 4:16 PM, Chandler Carruth <chandlerc at google.com> wrote:

> 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/1d6b10ed/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: SDAGfoldFMul.patch
Type: application/octet-stream
Size: 1041 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120907/1d6b10ed/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120907/1d6b10ed/attachment-0001.html>