review (S) for 6862863: C2 compiler fails in elide_copy()
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:
>> (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.
More information about the hotspot-compiler-dev