Request for reviews (XL): 6829193: JSR 292 needs to support SPARC
John Rose
john.r.rose at oracle.com
Wed Apr 21 12:32:17 PDT 2010
On Apr 21, 2010, at 11:13 AM, Tom Rodriguez wrote:
> Resending to list because it bounced.
>
> This change:
>
> + void get_cache_entry_pointer_at_bcp(Register cache, Register tmp, int bcp_offset, bool giant_index = false);
>
> seems spurious.
>
> I think the return_entry_for changes aren't really in the spirit of the design of the template interpreter. If invokedynamic has it's own strategy for lookup in the constant pool then it probably needs it's own return entry point.
Good point. And... note that the differences have to do with how the stack is popped. Nearly all of the complexity of return_entry goes away if the caller pre-pops the arguments. Before splitting return_entry, let's either do the pre-popping (making splitting moot) or decide that it's impractical.
> regcon_sll_ptr_by_con sure is convenient but its implementation is totally surprising and the name is long and unclear particularly since it has the same signature as regcon_sll_ptr but does something different. It desperately needs a comment in the hpp explaining what it does. I don't think dst should ever be optional, otherwise why it at all. This use:
>
> + __ add(Gargs, __ regcon_sll_ptr_by_con(G5_stack_move, LogBytesPerWord, G5_stack_move), Gargs);
>
> should just be sll_ptr. Actually given all the magic that the existing regcon_sll_ptr does, couldn't you shoehorn the new behaviour into it?
I would prefer that, but will defer to others on what is "too much magic".
> The by_con behaviour of returning the new regcon instead of the value return parameter seems better anyway.
> The switch in load_sized_value is too clever for my taste. How about just a switch on size_in_bytes with a test on is_signed where it's needed?
My high school writing teacher used to direct us, if we thought we had written something especially clever, to be ready to strike it out. Christian, if you break up the Overly Clever Switch in the sensible way Tom suggests, please break up the parallel one in the x86 code too.
-- John
More information about the hotspot-compiler-dev
mailing list