JDK10/RFR(M): 8172232: SPARC ISA/CPU feature detection is broken/insufficient (on Linux).

[Vladimir Kozlov]

On 10/2/17 8:52 AM, Patric Hedlin wrote:
> Hi Vladimir,
> 
> 
> On 09/29/2017 08:56 PM, Vladimir Kozlov wrote:
>> In general it is fine. Few notes.
>> You use ifdef DEBUG_SPARC_CAPS which is undefed at the beginning. Is it set by gcc by default?
>>
> I have not noticed any (obvious) convention in the code base for this case, when I have a entirely (file-) local, typically debug, definition that makes no sense to define except within a particular 
> file. I usually list those as undefines in the beginning of the file to make sure they are not exposed to the command line (the rationale being that they should not be of use if you are not actively 
> making changes to the particular file). And it sort of works as part of the local docs.

Got it. But in such situation we have other mechanisms to print information about CPUs.
I would suggest to use unified logging currently we use for this: -Xlog:os+cpu

http://hg.openjdk.java.net/jdk10/hs/file/58931d9b2260/src/hotspot/share/runtime/vm_version.cpp#l300

There are different levels of output and for your case you can use Debug or Trace level (default is Info).

Thanks,
Vladimir

> 
> Would it be an acceptable approach to add a comment like this:
> 
> /* NOTE: Enable the local define 'DEBUG_LINUX_SPARC_CAPS' below (or define it
>   *       from the command line) as an aid when updating the feature table.
> #define DEBUG_LINUX_SPARC_CAPS
>   */
> 
> Close to its first use (?). (I changed the name since it will be exposed outside the file.)
> 
>> Coding style for methods definitions - open parenthesis should be on the same line:
>>
>> +  bool match(const char* s) const
>> +  {
>>
> 
> Old habits die hard... and it's so much more readable ;)
> 
> /Patric
>> Thanks,
>> Vladimir
>>
>> On 9/29/17 6:08 AM, Patric Hedlin wrote:
>>> Dear all,
>>>
>>> I would like to ask for help to review the following change/update:
>>>
>>> Issue:  https://bugs.openjdk.java.net/browse/JDK-8172232
>>>
>>> Webrev: http://cr.openjdk.java.net/~phedlin/tr8172232/
>>>
>>>
>>> 8172232: SPARC ISA/CPU feature detection is broken/insufficient (on Linux).
>>>
>>>      Subsumes (duplicate) JDK-8186579: VM_Version::platform_features() needs update on linux-sparc.
>>>
>>>
>>> Caveat:
>>>
>>>      This update will introduce some redundancies into the code base, features and definitions
>>>      currently not used, addressed by subsequent bug or feature updates/patches. Fujitsu HW is
>>>      treated very conservatively.
>>>
>>>
>>> Testing:
>>>
>>>      JDK9/JDK10 local jtreg/hotspot
>>>
>>>
>>> Thanks to Adrian for additional test (and review) support.
>>>
>>> Tested-By: John Paul Adrian Glaubitz <glaubitz at physik.fu-berlin.de>
>>>
>>>
>>> Best regards,
>>> Patric
>>>
>