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

Vladimir Kozlov Vladimir.Kozlov at Sun.COM
Wed Aug 12 12:11:39 PDT 2009

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:
>>    ,n);
>>    ,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.


> tom
>> Thanks,
>> Vladimir

More information about the hotspot-compiler-dev mailing list