[llvm-commits] [llvm] r55886 - /llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

[Dan Gohman]

On Sep 8, 2008, at 11:03 AM, Bill Wendling wrote:

> On Mon, Sep 8, 2008 at 1:16 AM, Evan Cheng <evan.cheng at apple.com>  
> wrote:
>>
>> On Sep 7, 2008, at 5:35 PM, Bill Wendling wrote:
>>
>>> It's not a bug in that this wasn't being done. I was doing a static
>>> analysis of the DAG combiner and noticed that this wasn't doing what
>>> the comment claimed it should be doing. If this transformation is
>>> performed in the selection DAG code, then it should probably be
>>> removed from here entirely (along with the analogous "ADD"
>>> transformation). What do you think?
>>
>> No. The dag combiner xforms can still trigger. The issue is the
>> operand can be updated by something like ReplaceAllUsesWith which  
>> will
>> not transform the uses.
>>
> Okay, so should the dag combiner transforms be no-ops or actually do  
> something?

We want folding to happen both at getNode time and DAGCombiner time.
I think it would be possible to refactor the code to eliminate the
code redundancy though, by moving getNode's folding code out
into something like this:

   SDValue foldBinaryOp(ISD::NodeType Opcode, SDValue LHS, SDValue RHS);

It would return a null SDValue instead of creating a new node in the
case that it doesn't do a fold. Then getNode and DAGCombiner could both
use it. This would also have the advantage of making it easier to do
more folding at getNode time, which would make SelectionDAG more
efficient and make some existing optimizations more effective. What
do you think?

Dan