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

[Tom Rodriguez]

On Aug 11, 2009, at 9:49 PM, Vladimir Kozlov wrote:

> Tom Rodriguez wrote:
>> http://cr.openjdk.java.net/~never/6862863
>
> Few questions:
>
> 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?

> 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?

> 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.

tom

>
>
> Thanks,
> Vladimir