[llvm-commits] [llvm] r62533 - in /llvm/trunk: lib/CodeGen/SelectionDAG/DAGCombiner.cpp lib/CodeGen/SelectionDAG/SelectionDAG.cpp test/CodeGen/X86/pr3018.ll

[Chris Lattner]

On Jan 23, 2009, at 10:30 AM, Dan Gohman wrote:

>
> On Jan 21, 2009, at 11:26 PM, Chris Lattner wrote:
>
>> On Jan 19, 2009, at 1:44 PM, Dan Gohman wrote:
>>>
>>> +  // Iterate over all the existing uses of From. This  
>>> specifically avoids
>>> +  // visiting any new uses of From that arrise while the  
>>> replacement is
>>> +  // happening, because any such uses would be the result of CSE:  
>>> If an
>>> +  // existing node looks like From after one of its operands is  
>>> replaced
>>> +  // by To, we don't want to replace of all its users with To too.
>>> +  // See PR3018 for more info.
>>
>> Just from reading this, it isn't clear to me why this is safe.  Is  
>> it that any new uses get added to the start of the use list?  If  
>> so, please mention this.  Your implementation of this fix is  
>> dramatically better than anything I could come up with :)
>
> Thanks. It's as you suggest; I'll add some comments about this.
>
>>
>> One other random thing.  The RAUW methods now look like this:
> [...]
>>
>> It seems pretty silly to me to do the op_iterator scan right after  
>> scanning over the uses like that (from the compile time  
>> standpoint).  Would something like this work:?
> [...]
>
> Neat! It looks like it would work. It seems likely that
> if a node appears multiple times in another's use list,
> that the uses will be adjacent. I'll take a look at
> implementing this soon.

Thanks Dan!

-Chris