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

[Evan Cheng]

On Aug 10, 2009, at 3:02 PM, Dan Gohman wrote:

>
> 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

Ok, feel free to fix it better. :-)

Evan

> .
>
> Dan
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits