review (S) for 6862863: C2 compiler fails in elide_copy()

[Vladimir Kozlov]

Tom Rodriguez wrote:
> 
> On Aug 11, 2009, at 9:49 PM, Vladimir Kozlov wrote:
>> Why yank_if_dead() checks only in(1) if it is also dead
>> and not other inputs which could be dead also after old
>> node is removed? should we fix it?
> 
> It should never have more than 2 inputs and the control input is 
> irrelevant.  It's either a MachSpillCopy or a Con of some sort.  Maybe 
> there should be an assert for no more than 2 inputs?

The assert would be nice.

> 
>> Also from the current code it is not clear that at the end
>> it does next for this nreg:
>>
>>              regnd.map(nreg,n);
>>              value.map(nreg,val);
>>
>> (and corresponding nreg_lo mapping for register pair).
>> May be it needs additional comment in your changes.
> 
> It's got a comment to that effect already.  What kind of comment were 
> you thinking?

Something like this:

// Clear out a dead value mapped by the register to update
// the register->node mapping even if 'n' defines the same value.

> 
>> Also in the new method you use "regnd->at(nreg)"
>> when in other places regnd and value are passed as
>> references and "regnd[nreg]". Can you do the same?
> 
> The code uses a mix of NOde_List* and Node_List& which is pretty ugly.  
> I'd copied the signature of yank_if_dead which was already using 
> Node_List*.  I can change it to Node_List& if you like it better.

yank_if_dead() uses (*regnd)[old_reg]
I agree that it may be ugly but consistent.

Vladimir

> 
> tom
> 
>>
>>
>> Thanks,
>> Vladimir
>