6880034: Change 6420645 causes SIGBUS during deoptimisation at a safepoint on 64bit-SPARC

[Vladimir Kozlov]

Volker Simonis wrote:
 > I don't know why this strange encoding has been chosen for the 16
 > upper double registers in sparc.ad, but changing it to:
 >
 > reg_def R_D32x(SOC, SOC, Op_RegD,255, F32->as_VMReg()->next());
 > reg_def R_D32 (SOC, SOC, Op_RegD,  1, F32->as_VMReg());
 > reg_def R_D34x(SOC, SOC, Op_RegD,255, F34->as_VMReg()->next());
 > reg_def R_D34 (SOC, SOC, Op_RegD,  3, F34->as_VMReg());
 > ...
 > reg_def R_D62x(SOC, SOC, Op_RegD,255, F62->as_VMReg()->next());
 > reg_def R_D62 (SOC, SOC, Op_RegD, 31, F62->as_VMReg());
 >
 > which seems more natural to me, solved the SIGBUS issue and didn't
 > revealed any other problems in the tests which I run so far.
 >
 > Could you please comment on the proposed solution of changing the
 > VMReg numbering of F32-F62 or advice a better solution if you think
 > that the proposed one will not work in the general case?
 >
 > Thank you and best regards,
 > Volker

Your changes are correct but I would also swap definitions
to be more clear.

reg_def R_D32 (SOC, SOC, Op_RegD,  1, F32->as_VMReg());
reg_def R_D32x(SOC, SOC, Op_RegD,255, F32->as_VMReg()->next());
reg_def R_D34 (SOC, SOC, Op_RegD,  3, F34->as_VMReg());
reg_def R_D34x(SOC, SOC, Op_RegD,255, F34->as_VMReg()->next());

I looked on the history and originally it was

reg_def R_D32( SOC, SOC, Op_RegD,  1, 0 );
reg_def R_D32L(SOC, SOC, Op_RegD, 99, 0 );
reg_def R_D34( SOC, SOC, Op_RegD,  3, 0 );
reg_def R_D34L(SOC, SOC, Op_RegD, 99, 0 );

where R_D32L and R_D34L represented nonexisting F33 and F35
low 32 bits halves of D32 and D34.

Then the ordering of declarations was changed and introduced
the confusion:

reg_def R_D32x(SOC, SOC, Op_RegD,255, 0 );
reg_def R_D32 (SOC, SOC, Op_RegD,  1, 0 );
reg_def R_D34x(SOC, SOC, Op_RegD,255, 0 );
reg_def R_D34 (SOC, SOC, Op_RegD,  3, 0 );

Vladimir

Tom Rodriguez wrote:
>> The problem can be easily solved by switching back to the old
>> implementation of frame::oopmapreg_to_location(), but I assume there
>> was a rational behind the change and that the new implementation is
>> probably necessary for compressed oops (at least that's what the whole
>> change was all about). So I dug a little further and found that in my
>> opinion the root cause of the whole problem is the strange numbering
>> of the 16 upper double registers in sparc.ad. They are defined as
>> follows:
>>
>> reg_def R_D32x(SOC, SOC, Op_RegD,255, F32->as_VMReg());
>> reg_def R_D32 (SOC, SOC, Op_RegD,  1, F32->as_VMReg()->next());
>> reg_def R_D34x(SOC, SOC, Op_RegD,255, F34->as_VMReg());
>> reg_def R_D34 (SOC, SOC, Op_RegD,  3, F34->as_VMReg()->next());
>> ...
>> reg_def R_D62x(SOC, SOC, Op_RegD,255, F62->as_VMReg());
>> reg_def R_D62 (SOC, SOC, Op_RegD, 31, F62->as_VMReg()->next());
> 
> I don't really know why it's done this way.  It's certainly not 
> consistent with the others.  If it all works better I'd be ok with 
> correcting.
> 
>> PS: while I was hunting the error, I also stumbled across the
>> following code in RegisterSaver::save_live_registers():
>>
>>  // Save all the FP registers
>>  int offset = d00_offset;
>>  for( int i=0; i<64; i+=2 ) {
>>    FloatRegister f = as_FloatRegister(i);
>>    __ stf(FloatRegisterImpl::D,  f, SP, offset+STACK_BIAS);
>>    map->set_callee_saved(VMRegImpl::stack2reg(offset>>2), f->as_VMReg());
>>    if (true) {
>>      map->set_callee_saved(VMRegImpl::stack2reg((offset +
>> sizeof(float))>>2), f->as_VMReg()->next());
>>    }
>>    offset += sizeof(double);
>>  }
> 
> That would probably be ok too.
> 
> tom
> 
>>
>> In my opinion, this could be changed to:
>>
>>  // Save all the FP registers
>>  int offset = d00_offset;
>>  for( int i=0; i<64; i+=2 ) {
>>    FloatRegister f = as_FloatRegister(i);
>>    __ stf(FloatRegisterImpl::D,  f, SP, offset+STACK_BIAS);
>>    map->set_callee_saved(VMRegImpl::stack2reg(offset>>2), f->as_VMReg());
>>    if (i < 32) { // VS 2009-08-31: the 16 upper double registers
>> can't be used as floats anyway
>>      map->set_callee_saved(VMRegImpl::stack2reg((offset +
>> sizeof(float))>>2), f->as_VMReg()->next());
>>    }
>>    offset += sizeof(double);
>>  }
>>
>> because the 16 upper double registers can't be used as floats anyway.
>> Again, I didn't found any regression in my few tests. What do you
>> think?
>> <DeoptTest.java>
>