[CAUTION] RE: RFR(M): 8210497: [PPC64] Vector registers not saved across safepoint

[Schmidt, Lutz]

Hi Martin, 

your changes look good overall. I'm not a reviewer, so that judgement doesn't help you much. ��

I have found a few details you may want to consider:

src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp
@line 272: the new frame is pushed, but @line 275 comment says frame is not yet pushed.
Is there a reason why you need both, R30 and R31, as scratch in push_frame_reg_args_and_save_live_registers()? 

Regards,
Lutz


From: hotspot-compiler-dev <hotspot-compiler-dev-bounces at openjdk.java.net> on behalf of Götz Lindenmaier <goetz.lindenmaier at sap.com>
Date: Wednesday, 12. September 2018 at 12:12
To: "Doerr, Martin (martin.doerr at sap.com)" <martin.doerr at sap.com>, "'hotspot-compiler-dev at openjdk.java.net'" <hotspot-compiler-dev at openjdk.java.net>, "Michihiro Horie (HORIE at jp.ibm.com)" <HORIE at jp.ibm.com>, Gustavo Romero <gromero at linux.vnet.ibm.com>
Subject: [CAUTION] RE: RFR(M): 8210497: [PPC64] Vector registers not saved across safepoint

Hi Martin, 
 
I had a look at your fix and it looks good. 
I currently can’t judge though whether even more 
vector support is needed in some other place. 
But this is not subject of this fix I guess.
 
Reviewed.
 
Best regards,
  Geotz.
 
PS: I would appreciate if you put the ‘save_registers’
argument on a line of it’s own whereever this is done for
all other arguments.  No new webrev needed.
 
 
From: Doerr, Martin 
Sent: Freitag, 7. September 2018 18:12
To: 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-dev at openjdk.java.net>; Michihiro Horie (HORIE at jp.ibm.com) <HORIE at jp.ibm.com>; Gustavo Romero <gromero at linux.vnet.ibm.com>; Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
Subject: RFR(M): 8210497: [PPC64] Vector registers not saved across safepoint
 
Hi,
 
we noticed that the RegisterSaver misses code to save and restore the vector registers on PPC64. I’d like to fix that.
 
Bug:
https://bugs.openjdk.java.net/browse/JDK-8210497
 
Webrev:
http://cr.openjdk.java.net/~mdoerr/8210497_PPC64_save_CR/webrev.00/
 
This webrev already fixes the following tests when JDK-8208171 webrev.03 is applied:
compiler/runtime/safepoints/TestRegisterRestoring.java
compiler/runtime/Test7196199.java
 
I’ll try to test the OopMap part. This may be tricky.
 
Best regards,
Martin