RFR(M): 8153076: Atomic::* on Solaris should use inline assembly instead of .il files

[David Holmes]

On 11/04/2016 9:57 PM, Claes Redestad wrote:
> Not really a qualified reviewer in this area, but I think this looks great!
>
> Shouldn't the remaining solaris 32-bit build support in the (old)
> makefiles and sources be purged altogether? Is there any point in
> holding on to things like solaris_x86_32.s (which in turn is referencing
> non-existent sources like os_solaris_i486.cpp)?

I think the last time this was raised it was pointed out that non-Oracle 
JDK builders still build 32-bit Solaris. Otherwise eradicating 32-bit 
Solris support would be a separate RFE.

David

> Thanks!
>
> /Claes
>
> On 2016-04-11 11:19, Erik Österlund wrote:
>> Hi Dean,
>>
>> Changing the input to a memory operand did indeed work better. Thanks
>> for the great idea.
>> I updated the x86 code with cleaner assembly.
>>
>> Updated webrev: http://cr.openjdk.java.net/~eosterlund/8153076/webrev.01/
>>
>> Thanks,
>> /Erik
>>
>> On 2016-04-09 02:20, Dean Long wrote:
>>> Hi Erik.  Does this work any better?
>>>
>>>   __asm__ volatile ("lock; addl $1,%0" : "+m" (*dest) : : "cc",
>>> "memory");
>>>
>>>
>>> dl
>>>
>>> On 4/8/2016 4:52 AM, Erik Österlund wrote:
>>>> Hi,
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8153076
>>>> Webrev: http://cr.openjdk.java.net/~eosterlund/8153076/webrev.00/
>>>>
>>>> On Solaris, the Atomic class puts its assembly in a separate .il
>>>> file, then the code blobs are declared as extern "C" and called from
>>>> Atomic. More modern GCC-style inline assembly is now supported, and
>>>> we should use that instead. It works fine the way it is, but making
>>>> calls just to get an instruction that could be inlined, seems
>>>> suboptimal.
>>>>
>>>> There are two important things to note:
>>>>
>>>> 1) I have removed all the 32 bit code from solaris Atomic, and weird
>>>> combinations with GCC and compiler1. JDK9 is only supported on 64
>>>> bit SPARC and x86, building using Solaris Studio. Since it is using
>>>> GCC-style inline assembly, the code should work with GCC too, but I
>>>> have not tested that as it is not a supported way of building
>>>> OpenJDK. I personally do not think we should spend too much time on
>>>> supporting unsupported code.
>>>>
>>>> 2) There appears to be a bug in Solaris Studio for emitting memory
>>>> accesses on x86.
>>>>
>>>> The following code:
>>>>
>>>> inline void Atomic::inc    (volatile jint*     dest) {
>>>>   __asm__ volatile ("lock; addl $1,(%0)" :
>>>>                     : "r" (dest) : "cc", "memory");
>>>> }
>>>>
>>>> generated the following in HandleMark::initialize(Thread* thread) on
>>>> Solaris, x64, using SS12.4:
>>>>
>>>> lock addl $1, (%eax)
>>>>
>>>> Note the %eax instead of %rax. It is wrong and caused SIGSEGV as the
>>>> address was truncated, and libraries are not put within the first 4
>>>> GB of VA.
>>>>
>>>> I worked around the issue by assigning the address operands to a
>>>> fixed register, rdx. Being awkward with the choice of registers is
>>>> still better than having to make calls. This will have to be fixed,
>>>> once the solaris studio bug is fixed.
>>>>
>>>> Note that for SPARC there were no issues like this and the generated
>>>> code is fine.
>>>>
>>>> Testing: JPRT
>>>>
>>>> I need a sponsor to push this.
>>>>
>>>> Thanks,
>>>> /Erik
>>>
>>
>