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

[Dan Gohman]

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.

Dan