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

Volker Simonis volker.simonis at gmail.com
Fri Oct 2 10:35:03 PDT 2009

Hi Vladimir, Tom,

thank you for your evaluation.

You can find a webrev for this change at:

http://www.progdoc.de/webrev/6880034/

Could you please be so kind to review and submit it in case it's ok?

Thanks, Volker

On 9/30/09, Vladimir Kozlov <Vladimir.Kozlov at sun.com> wrote:

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: > > regdef RD32x(SOC, SOC, OpRegD,255, F32->asVMReg()->next()); > regdef RD32 (SOC, SOC, OpRegD, 1, F32->asVMReg()); > regdef RD34x(SOC, SOC, OpRegD,255, F34->asVMReg()->next()); > regdef RD34 (SOC, SOC, OpRegD, 3, F34->asVMReg()); > ... > regdef RD62x(SOC, SOC, OpRegD,255, F62->asVMReg()->next()); > regdef RD62 (SOC, SOC, OpRegD, 31, F62->asVMReg()); > > 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. regdef RD32 (SOC, SOC, OpRegD, 1, F32->asVMReg()); regdef RD32x(SOC, SOC, OpRegD,255, F32->asVMReg()->next()); regdef RD34 (SOC, SOC, OpRegD, 3, F34->asVMReg()); regdef RD34x(SOC, SOC, OpRegD,255, F34->asVMReg()->next()); I looked on the history and originally it was regdef RD32( SOC, SOC, OpRegD, 1, 0 ); regdef RD32L(SOC, SOC, OpRegD, 99, 0 ); regdef RD34( SOC, SOC, OpRegD, 3, 0 ); regdef RD34L(SOC, SOC, OpRegD, 99, 0 ); where RD32L and RD34L represented nonexisting F33 and F35 low 32 bits halves of D32 and D34. Then the ordering of declarations was changed and introduced the confusion: regdef RD32x(SOC, SOC, OpRegD,255, 0 ); regdef RD32 (SOC, SOC, OpRegD, 1, 0 ); regdef RD34x(SOC, SOC, OpRegD,255, 0 ); regdef RD34 (SOC, SOC, OpRegD, 3, 0 ); Vladimir Tom Rodriguez wrote: > > > The problem can be easily solved by switching back to the old > > implementation of frame::oopmapregtolocation(), 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: > > > > regdef RD32x(SOC, SOC, OpRegD,255, F32->asVMReg()); > > regdef RD32 (SOC, SOC, OpRegD, 1, F32->asVMReg()->next()); > > regdef RD34x(SOC, SOC, OpRegD,255, F34->asVMReg()); > > regdef RD34 (SOC, SOC, OpRegD, 3, F34->asVMReg()->next()); > > ... > > regdef RD62x(SOC, SOC, OpRegD,255, F62->asVMReg()); > > regdef RD62 (SOC, SOC, OpRegD, 31, F62->asVMReg()->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::saveliveregisters(): > > > > // Save all the FP registers > > int offset = d00offset; > > for( int i=0; i<64; i+=2 ) {_ _> > FloatRegister f = asFloatRegister(i); > > _ stf(FloatRegisterImpl::D, f, SP, offset+STACKBIAS); > > map->setcalleesaved(VMRegImpl::stack2reg(offset>>2), f->asVMReg()); > > if (true) { > > map->setcalleesaved(VMRegImpl::stack2reg((offset + > > sizeof(float))>>2), f->asVMReg()->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 = d00offset; > > for( int i=0; i<64; i+=2 ) {_ _> > FloatRegister f = asFloatRegister(i); > > _ stf(FloatRegisterImpl::D, f, SP, offset+STACKBIAS); > > map->setcalleesaved(VMRegImpl::stack2reg(offset>>2), f->asVMReg()); > > if (i < 32) { // VS 2009-08-31: the 16 upper double registers_ _> > can't be used as floats anyway > > map->setcalleesaved(VMRegImpl::stack2reg((offset + > > sizeof(float))>>2), f->asVMReg()->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> > > > >

More information about the hotspot-compiler-dev mailing list