RFR(M): 8195112: x86 (32 bit): implementation for Thread-local handshakes

[Robbin Ehn]

Hi Martin,

On 02/19/2018 03:54 PM, Lindenmaier, Goetz wrote:
> Hi Martin,
> 
> I had a look at your change.
> 
> c1_LIRAssembler_x86.cpp:
> Address(poll_addr, in_bytes(Thread::polling_page_offset()
> Why do you need to use in_bytes to cast away ByteSize?
> In the debug build, there is a proper Address constructor, and
> in opt it should be pointless.  In other places, you don't use it
> either.
> 
> macroAssembler_x86.hpp
> Maybe you could coment above the definition of safepoint_poo()

Maybe add an assert here for 64-bit that it really is r15.

> // If thread_reg is != noreg the code assumes the register passed contains
> // the thread.
> 
> sharedRuntime_x86_32.cpp:
> Please break comment to shorter lines.
> 
> Sometimes, you protect code by _LP64, sometimes by AMD64.
> Why? (I prefer AMD64 to protect code that depends on the architectural
> extension for 64-bit ...).
> Instead of ifndef AMD64 you can use ifdef IA32 which you do
> at other places :)
> 
> I think the submission forest is good to show you didn't break
> X86_64. So I think if it passes there you can push it.

Yes that would be good.

> 
> Reviewed.

+1

->

> 
> Best regards,
>    Goetz.
> 
> -----Original Message-----
> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-bounces at openjdk.java.net] On Behalf Of Doerr, Martin
> Sent: Monday, February 19, 2018 1:15 PM
> To: Robbin Ehn <robbin.ehn at oracle.com>; hotspot-runtime-dev at openjdk.java.net
> Subject: [CAUTION] RE: RFR(M): 8195112: x86 (32 bit): implementation for Thread-local handshakes
> 
> Hi,
> 
> I have disabled ThreadLocalHandshakes by default on linux 32 bit.
> After some performance measurements, I have also switched it off on Windows 32 bit if the server compiler is not active (arguments.cpp) which is the default.
> C1 performance is affected if enabled. C2 and tiered performance look ok.
> 
> So on 32 bit, ThreadLocalHandshakes are only used on Windows when enabling server mode with this updated version.
> 
> Please review:
> http://cr.openjdk.java.net/~mdoerr/8195112_x86_local_poll/webrev.01/
> 
> @Robbin: I guess the submission forest is not helpful for 32 bit?

Unfortunately no.

> Will I still need a sponsor for it?

I don't know.

Thanks for fixing, Robbin

> 
> Best regards,
> Martin
> 
> 
> -----Original Message-----
> From: Andrew Haley [mailto:aph at redhat.com]
> Sent: Donnerstag, 8. Februar 2018 17:28
> To: Doerr, Martin <martin.doerr at sap.com>; David Holmes <david.holmes at oracle.com>; Robbin Ehn <robbin.ehn at oracle.com>; hotspot-runtime-dev at openjdk.java.net
> Subject: Re: RFR(M): 8195112: x86 (32 bit): implementation for Thread-local handshakes
> 
> On 08/02/18 13:04, Doerr, Martin wrote:
>> Are you planning to fix it soon?
>> If so, we can delay my change until it was pushed.
> 
> I was, but I keep getting interrupted by urgent things.
> 
>> Otherwise, we can keep ThreadLocalHandshakes off by default on linux x86 32 bit.
>> That would be ok for us, too. We'd just like the feature to be available.
> 
> Sure, do that.  Thanks.
>