[10] RFR(S): 8185572: Enable AssumeMP by default on SPARC machines

[coleen.phillimore at oracle.com]

This looks good.  It'll be nice when the whole thing gets cleaned up.

Coleen


On 8/10/17 3:11 PM, Vladimir Kozlov wrote:
> Looks good. I think someone from Runtime have to review it too.
>
> Thanks,
> Vladimir
>
> On 8/10/17 11:55 AM, Poonam Parhar wrote:
>> Updated the webrev with comments:
>> http://cr.openjdk.java.net/~poonam/8185572/webrev.01/
>>
>> Thanks,
>> Poonam
>>
>>
>>> -----Original Message-----
>>> From: Vladimir Kozlov
>>> Sent: Thursday, August 10, 2017 11:24 AM
>>> To: hotspot-compiler-dev at openjdk.java.net; Poonam Bajaj Parhar
>>> Cc: hotspot-runtime-dev at openjdk.java.net runtime
>>> Subject: Re: [10] RFR(S): 8185572: Enable AssumeMP by default on SPARC
>>> machines
>>>
>>> Poonam,
>>>
>>> I mean to add a small (one or two sentences) comment to the code. Some
>>> thing like next but may better wording:
>>>
>>> +  if (FLAG_IS_DEFAULT(AssumeMP)) {
>>> +    // BIS instructions require 'membar' instruction regardless number
>>> of CPU.
>>> +    // Otherwise in virtualized/container environments which use only
>>> 1
>>> cpu BIS instructions may produce incorrect results.
>>> +    FLAG_SET_DEFAULT(AssumeMP, true);
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 8/10/17 10:44 AM, Poonam Parhar wrote:
>>>> Thanks Vladimir.
>>>>
>>>> Since the SPARC machines are always multi-cores, we can safely set
>>> AssumeMP to true on these.
>>>>
>>>> Adding my comments from the previous mail here again for better
>>> readability:
>>>> -------------------------------------
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8185572: Enable
>>> AssumeMP
>>>> by default on SPARC machines
>>>> Webrev: http://cr.openjdk.java.net/~poonam/8185572/webrev.00/
>>>>
>>>> This change enables AssumeMP by default on SPARC machines. On Sparc
>>> T7, to finalize BIS instructions the server compiler needs to add a
>>> 'membar' instruction at the end. But the generation of 'membar' is
>>> guarded by os::is_MP(), and os::is_MP() returns false when there is a
>>> single cpu available on the system. Now, in virtualized/container
>>> environments, the number of processors allocated to a virtual machine
>>> can dynamically change during the application runtime. That could lead
>>> to incorrect generation of BIS instructions and can cause JVM crashes.
>>> Enabling AssumeMP makes is_MP() always return true on SPARC systems.
>>>>
>>>> In future, we may consider making generation of 'membar'
>>> unconditional with the enhancement request:
>>> https://bugs.openjdk.java.net/browse/JDK-8150715.
>>>>
>>>> Thanks,
>>>> Poonam
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Vladimir Kozlov
>>>>> Sent: Thursday, August 10, 2017 9:47 AM
>>>>> To: Poonam Parhar; hotspot-compiler-dev at openjdk.java.net
>>>>> Cc: hotspot-runtime-dev at openjdk.java.net runtime
>>>>> Subject: Re: [10] RFR(S): 8185572: Enable AssumeMP by default on
>>>>> SPARC machines
>>>>>
>>>>> CCing to Runtime.
>>>>>
>>>>> Can you add comment explaining why it set to true on SPARC?
>>>>>
>>>>> Thanks,
>>>>> Vladimir
>>>>>
>>>>> On 8/10/17 6:26 AM, Poonam Parhar wrote:
>>>>>> Hello,
>>>>>>
>>>>>> Please review this simple patch:
>>>>>>
>>>>>> Bug:_JDK-8185572_<https://bugs.openjdk.java.net/browse/JDK-
>>>>> 8185572>:En
>>>>>> able
>>>>>> AssumeMP by default on SPARC machines
>>>>>>
>>>>>> Webrev:http://cr.openjdk.java.net/~poonam/8185572/webrev.00/
>>>>>>
>>>>>> This change enables AssumeMP by default on SPARC machines. On Sparc
>>>>>> T7, to finalize BIS instructions the server compiler needs toadd
>>>>>> a'membar'instruction at the end.But the generation of'membar'is
>>>>>> guarded byos::is_MP(), andos::is_MP()returns false when there isa
>>>>>> singlecpu available on the system.
>>>>>> Now,invirtualized/containerenvironments, the number
>>>>>> ofprocessorsallocated to a virtual machine can dynamically change
>>>>>> during the application runtime.That could lead to incorrect
>>>>> generation
>>>>>> of BIS instructions and can cause JVM crashes.Enabling AssumeMP
>>>>>> makes
>>>>>> is_MP() always return true on SPARC systems.
>>>>>>
>>>>>> In future, we may consider makinggeneration of'membar'unconditional
>>>>>> withtheenhancementrequest:_JDK-
>>>>> 8150715_<https://bugs.openjdk.java.net/browse/JDK-8150715>.
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Poonam
>>>>>>