RFR(S): 8246377: [PPC64] Further improvements for assembler stop function

Doerr, Martin martin.doerr at sap.com
Wed Jun 3 18:16:00 UTC 2020


Hi Xin,

thanks a lot for looking at it.

> You actually make TDI a variable-length instruction.  Why not just emit
> "(uintptr_t)msg" no matter what?
I don't make TDI a variable-length instruction. It's still 4 Byte like all instructions on PPC.
And MacroAssembler::stop is already variable-length. C2's HaltNode implementation too. I don't see any issue with that.
Why should we waste space and confuse disassembly if we have no information to add?

> Particularly, "enum stop_msg_present = -0x8000" is correct but subtle.  I'd
> like to learn from other reviewers.
Yeah, looks a bit ugly. I would have preferred 0x8000 in order to set the highest bit of the SI16 field, but it's signed so I need to use the negative value to avoid assertions complaining about integer overflow.

I think using this bit also makes the recognition of the instruction a bit more reliable before we read the const char* from the instruction stream.

Btw. I have improved formatting in macroAssembler_ppc.hpp and in the os files as requested by Götz. Updated webrev in place.

Are you ok with it? Can I add you as 2nd reviewer (no Reviewer status needed for that)?

Best regards,
Martin


> -----Original Message-----
> From: Liu, Xin <xxinliu at amazon.com>
> Sent: Mittwoch, 3. Juni 2020 01:56
> To: Doerr, Martin <martin.doerr at sap.com>; hotspot-runtime-
> dev at openjdk.java.net; Schmidt, Lutz <lutz.schmidt at sap.com>; Andrew
> Haley (aph at redhat.com) <aph at redhat.com>
> Subject: Re: RFR(S): 8246377: [PPC64] Further improvements for assembler
> stop function
> 
> Hi, Martin,
> 
> I read it and it looks good to me, but it still needs a real reviewer approve.
> 
> You actually make TDI a variable-length instruction.  Why not just emit
> "(uintptr_t)msg" no matter what?
> Particularly, "enum stop_msg_present = -0x8000" is correct but subtle.  I'd
> like to learn from other reviewers.
> 
> Thanks,
> --lx
> 
> 
> From: "Doerr, Martin" <martin.doerr at sap.com>
> Date: Tuesday, June 2, 2020 at 3:35 PM
> To: hotspot-runtime-dev <hotspot-runtime-dev at openjdk.java.net>,
> "Schmidt, Lutz" <lutz.schmidt at sap.com>, "Andrew Haley
> (aph at redhat.com)" <aph at redhat.com>, "Liu, Xin" <xxinliu at amazon.com>
> Subject: [EXTERNAL] RFR(S): 8246377: [PPC64] Further improvements for
> assembler stop function
> 
> CAUTION: This email originated from outside of the organization. Do not click
> links or open attachments unless you can confirm the sender and know the
> content is safe.
> 
> Hi,
> 
> I’d like to have the improvements which we discussed for AArch64 also on
> PPC64 (+ a little more).
> 
> Issue with details:
> https://bugs.openjdk.java.net/browse/JDK-8246377
> 
> Webrev:
> http://cr.openjdk.java.net/~mdoerr/8246377_ppc64_improve_stop/webrev
> .00/
> 
> Please review.
> 
> Best regards,
> Martin
> 



More information about the hotspot-runtime-dev mailing list