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