RFR: 8192123: Zero should use compiler built-ins for atomics on linux-arm

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8192123: Zero should use compiler built-ins for atomics on linux-arm

John Paul Adrian Glaubitz
Hi!

Like it has already been done for linux-m68k in JDK-8187227, Zero should also be
patched on linux-arm to use compiler built-ins for the atomic functions in
hotspot/os_cpu/linux_zero/assembler_linux_zero.cpp.

Since the compiler built-ins for cmpxchg() work exactly like the current ARM-specific
implementation in Zero, there is no need anymore to carry our own implementation but
rather rely on gcc which helps simplifying the code.

I have successfully tested this change in the following targets:

- Linux ARMv4T
- Linux ARMv7

My change also removes a single empty line to make the formatting more
consistent. Hope this is ok.

The webrev can be found in [1].

Thanks,
Adrian

> [1] http://cr.openjdk.java.net/~glaubitz/8192123/webrev.00/

--
  .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - [hidden email]
`. `'   Freie Universitaet Berlin - [hidden email]
   `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8192123: Zero should use compiler built-ins for atomics on linux-arm

Andrew Haley
On 29/11/17 14:08, John Paul Adrian Glaubitz wrote:

> Since the compiler built-ins for cmpxchg() work exactly like the current ARM-specific
> implementation in Zero, there is no need anymore to carry our own implementation but
> rather rely on gcc which helps simplifying the code.
>
> I have successfully tested this change in the following targets:
>
> - Linux ARMv4T
> - Linux ARMv7
>
> My change also removes a single empty line to make the formatting more
> consistent. Hope this is ok.

Hmm.  This doesn't fix a bug, and there's a non-zero probability of
introducing one.  I guess it should be OK, but this is a combination
of performance improvement (possibly) and cleanup.

--
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8192123: Zero should use compiler built-ins for atomics on linux-arm

coleen.phillimore


On 11/29/17 10:04 AM, Andrew Haley wrote:

> On 29/11/17 14:08, John Paul Adrian Glaubitz wrote:
>> Since the compiler built-ins for cmpxchg() work exactly like the current ARM-specific
>> implementation in Zero, there is no need anymore to carry our own implementation but
>> rather rely on gcc which helps simplifying the code.
>>
>> I have successfully tested this change in the following targets:
>>
>> - Linux ARMv4T
>> - Linux ARMv7
>>
>> My change also removes a single empty line to make the formatting more
>> consistent. Hope this is ok.
> Hmm.  This doesn't fix a bug, and there's a non-zero probability of
> introducing one.  I guess it should be OK, but this is a combination
> of performance improvement (possibly) and cleanup.
>
Please save this change for JDK 11.
Thanks,
Coleen
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8192123: Zero should use compiler built-ins for atomics on linux-arm

John Paul Adrian Glaubitz
On 11/29/2017 04:39 PM, [hidden email] wrote:
>> Hmm.  This doesn't fix a bug, and there's a non-zero probability of
>> introducing one.  I guess it should be OK, but this is a combination
>> of performance improvement (possibly) and cleanup.
>>
> Please save this change for JDK 11.

I just made the same obvious change as I did for m68k. Is there really
a risk of introducing a regression?

It only affects Zero after all which is not officially supported, is it?

The supported ARMv7 Server build is not affected as this change is
below the linux_zero directory.

Adrian

--
  .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - [hidden email]
`. `'   Freie Universitaet Berlin - [hidden email]
   `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8192123: Zero should use compiler built-ins for atomics on linux-arm

Andrew Haley
On 29/11/17 15:45, John Paul Adrian Glaubitz wrote:
> On 11/29/2017 04:39 PM, [hidden email] wrote:
>>> Hmm.  This doesn't fix a bug, and there's a non-zero probability of
>>> introducing one.  I guess it should be OK, but this is a combination
>>> of performance improvement (possibly) and cleanup.
>>>
>> Please save this change for JDK 11.
>
> I just made the same obvious change as I did for m68k. Is there really
> a risk of introducing a regression?

There's always a risk with any change.  In this case, we know that the
kernel builtin is very stable, and we don't know that every version of
GCC will always use the kernel builtin.

> It only affects Zero after all which is not officially supported, is it?
>
> The supported ARMv7 Server build is not affected as this change is
> below the linux_zero directory.

Yea, but there's still a risk, supported or not.  It can surely wait
until 11.

--
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671