Request for reviews (XS): 6879902: CTW failure jdk6_18/hotspot/src/cpu/sparc/vm/assembler_sparc.hpp:845
Christian Thalinger
Christian.Thalinger at Sun.COM
Fri Oct 2 03:32:53 PDT 2009
On Tue, 2009-09-22 at 10:10 -0700, Tom Rodriguez wrote:
> We could convert arg_slot/next_arg_slot to return RegisterOrConstant
> and fix this uniformly. The existing code assumes that the offsets
> fit in 32 bit mode but don't in 64 bit which seems dubious. arg_slot
> could then be used everywhere, as long as we add an assert that
> value_offset_in_bytes == 0.
Just for the record... I have already prepared another webrev but did
had time yet to post it:
http://cr.openjdk.java.net/~twisti/6879902/webrev.02/
and Tom had a look at it:
I looked at this again and I don't think that stf should be using
arg_slot. c2i only uses arg_slot/next_arg_slot for loads which is
correct since we should be moving it from interpreter layout to
compiled layout. Using it for stores is wrong. After Vladimir's
comment I thought that other compiled code was already using arg_slot
for stores so it wasn't any worse but now I can see that it's not
true. I think the original fix of just correcting the stf code was
the way to go. It can use RegisterOrConstant with ensure_rs2. We can
keep the other changes or not depending on how you feel about them.
They generate better code for 64-bit and deals with the simm13 issue
in general which I like. If you keep them I'd suggest using
ensure_rs2 in you arg_slot routines instead of hand coding that. I
wouldn't mind renaming ensure_rs2 since it's such a useless name.
-- Christian
More information about the hotspot-compiler-dev
mailing list