RFR for JDK-8165482 java in ldoms, with cpu-arch=generic has problems

[David Holmes]

Hi Martin,

On 30/09/2016 8:03 PM, Martin Walsh wrote:
> On 23/09/2016 14:34, Martin Walsh wrote:
>> On 20/09/2016 08:42, Erik Joelsson wrote:
>>> Hello,
>>>
>>>
>>> On 2016-09-20 03:28, David Holmes wrote:
>>>> Hi Martin,
>>>>
>>>> Build changes must be reviewed by the build team - now cc'd
>>>>
>>> Thanks for forwarding David, I can't keep up with all the lists to find
>>> these unless posted to build-dev.
>>>> On 20/09/2016 12:16 AM, Martin Walsh wrote:
>>>>> Could I get a code review for the following bug:
>>>>>
>>>>> JDK-8165482 java in ldoms, with cpu-arch=generic has problems
>>>>>
>>>>> Webrev of the changes is available here:
>>>>>
>>>>> http://cr.openjdk.java.net/~mwalsh/JDK-8165482/
>>>>
>>>> What is the devinfo library? Is it part of the normal Solaris
>>>> installation, or does it need to be installed specifically? Is it
>>>> available in our official build toolkits?
>>> I did a bit of digging. It's part "system/libraries" so should be pretty
>>> standard. That package is in the devkit and I verified that
>>> libdevinfo.so is there too.
>>>
>>> Configure change looks fine. Just remember to also push the closed
>>> generated-configure.sh as David said.
>>>
>>> /Erik
>>>>
>>>> Will checking the prom prior to using kstat change any of the values
>>>> we currently see? (other than the generic case being fixed of course).
>>
>> It shouldn't, as SPARC64 seems to be reserved for Fujitsu SPARC
>> machines.  However, to air on the side of caution I will investigate
>> further and follow-up shortly.
>
> So, after further deliberation I have decided that although utilising
> the PROM may work on some occasions, it is not the correct solution.
>
> All Oracle SPARC LDOMS use the "sun4-cpu" string as the CPU
> implementation, therefore I think the best short term fix is to add an
> additional match that checks for this string.  Long term, this could do
> with a re-write, but that is a JDK10 project.
>
> This updated fix also means there are no build changes.
>
> Updated the webrev accordingly.
>
> http://cr.openjdk.java.net/~mwalsh/JDK-8165482/

These new changes seem okay to me.

Only one nit in src/os_cpu/solaris_sparc/vm/vm_version_solaris_sparc.cpp

  467             }
  468           }
  469   }
  470       }

the indent on line 469 is wrong.

Thanks,
David
-----
> Thanks,
>
> Martin
>