[llvm-commits] [llvm] r78594 - in /llvm/trunk: lib/Target/ARM/ARMISelDAGToDAG.cpp test/CodeGen/Thumb2/2009-08-10-ISelBug.ll

[Dan Gohman]

On Aug 10, 2009, at 2:38 PM, Evan Cheng wrote:


>
> On Aug 10, 2009, at 2:00 PM, Eli Friedman wrote:
>
>
>> On Mon, Aug 10, 2009 at 1:25 PM, Evan Cheng<evan.cheng at apple.com>
>>
>> wrote:
>>
>>> Author: evancheng
>>>
>>> Date: Mon Aug 10 15:25:59 2009
>>>
>>> New Revision: 78594
>>>
>>>
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=78594&view=rev
>>>
>>> Log:
>>>
>>> Handle the constantfp created during post-legalization dag combiner
>>>
>>> phase.
>>>
>>
>>
>> AFAIK, dagcombine isn't  supposed to create illegal constantfp nodes
>>
>> post-legalize... perhaps it would be better to fix the issue there?
>>
>
> Yes and no. These transformations are somewhat essential so perhaps
> the *right* solution is to emit loads from constantpool instead of
> constantfp nodes.

In the included testcase, the first DAGCombine run is missing an
opportunity to fold an "x-x", which gets exposed after some other nodes
get combined away. The second DAGCombine run then folds the "x-x" into
the 0, which is folded into the sint_to_fp to form the 0.0 node.

It seems that the bug here is that both instcombine and the first
dagcombine phase have missed a folding opportunity. If either of those
two did that, then it wouldn't be essential for the second phase to
break the rules just to do the fold.

> On the other hand, ARM isel is able to handle
> constantfp nodes. So it seems perfectly ok to consider them legal.

Not really. As of your patch, it handles them be manually legalizing  
them,
which it shouldn't have to do.

Dan