RFR (S): 8019922 : PPC64 (part 8): Implement Linux/PPC64 support in HotSpot makefiles

[Vladimir Kozlov]

Tests passed (included our ppc builds and testing).

Vladimir

On 7/8/13 4:37 PM, Vladimir Kozlov wrote:
> Looks good.
>
> I started JPRT testing. Hopefully will get result tonight.
>
> Thanks,
> Vladimir
>
> On 7/8/13 8:42 AM, Volker Simonis wrote:
>> Hi David,
>>
>> thank you for the review. I prepared a new webrev according to your suggestions:
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/8019922.v2/
>>
>> Please find my comments inline:
>>
>> Thank you and best regards,
>> Volker
>>
>> On Fri, Jul 5, 2013 at 7:21 AM, David Holmes <david.holmes at oracle.com> wrote:
>>> Hi Volker,
>>>
>>> This all looks fine to me. A couple of minor comments below.
>>>
>>> I will also test it but I don't expect to see any conflicts with our other
>>> ports.
>>>
>>> Thanks,
>>> David
>>> -----
>>>
>>> make/defs.make
>>>
>>> Minor observation: As BUILDARCH defaults to SRCARCH you can emulate the
>>> sparcv9 code and reduce this:
>>>
>>>   288   ifeq ($(BUILDARCH), ppc)
>>>   289     ifdef LP64
>>>   290       BUILDARCH = ppc64
>>>   291     else
>>>   292       BUILDARCH = ppc
>>>   293     endif
>>>   294   endif
>>>
>>> to
>>>
>>>   288   ifeq ($(BUILDARCH), ppc)
>>>   289     ifdef LP64
>>>   290       BUILDARCH = ppc64
>>>   291     endif
>>>   292   endif
>>>
>>> ---
>>
>> fixed as suggested.
>>
>>>
>>> make/linux/makefiles/defs.make
>>>
>>> I note you have:
>>>
>>>   HS_ARCH          = ppc
>>>
>>> and this is only used with the SA for which we presently have:
>>>
>>> ADD_SA_BINARIES/ppc   =
>>>
>>> but you also added:
>>>
>>> ADD_SA_BINARIES/ppc64 =
>>>
>>> so was the HS_ARCH setting a typo?
>>>
>>> ---
>>
>> Well, I did it in the same way as the other architectures did it:)
>>
>> Moreover, HS_ARCH is also used in make/Makefile.
>>
>>>
>>> make/linux/makefiles/gcc.make
>>>
>>> You added:
>>>
>>> ARCHFLAG/ppc64   =  -m64
>>>
>>> which is fine, but then in ppc64.make you also have:
>>>
>>> CFLAGS += -m64
>>> LFLAGS_VM += -m64
>>> AOUT_FLAGS += -m64
>>>
>>> which seems redundant given gcc.make has:
>>>
>>>   186 CFLAGS     += $(ARCHFLAG)
>>>   187 AOUT_FLAGS += $(ARCHFLAG)
>>>   188 LFLAGS     += $(ARCHFLAG)
>>>   189 ASFLAGS    += $(ARCHFLAG)
>>>
>>
>> You're right. I removed the '-m64' settings from ppc64.make
>> Also, 'LAUNCHERFLAGS' isn't used anymore so I removed it from
>> pp464.make as well.
>> The same applies to 'AOUT_FLAGS' which isn't necessary for our build.
>>
>>> Also the initial copyright line seems suspect:
>>>
>>>     2 # Copyright (c) 2004, 2011, Oracle and/or its affiliates. All rights
>>> reserved.
>>>
>>> Not sure how a new file can have a 2004-2011 copyright year range ??
>>>
>>
>> That was a copy-paste issue. After the lengthy discussions below I
>> changed it to "2004, 2013" to keep things simple:)
>>
>>>
>>>
>>> On 5/07/2013 2:07 AM, Volker Simonis wrote:
>>>>
>>>> Hi,
>>>>
>>>> can sombody please review this change. It implements the relevant
>>>> HotSpot make changes to build the HotSpot on Linux/PPC64 and shouldn't
>>>> touch any existing platforms (but of course testing it on your closed
>>>> platforms - especially ppc - is probably necessary and much
>>>> appreciated).
>>>>
>>>> It also contains two small fixes for the CORE build (one is a type and
>>>> the other is necessary to accomodate to the changes in "8008772:
>>>> remove gamma launcher") in 'make/Makefile' for which I didn't wanted
>>>> to open an extra bug for.
>>>>
>>>> Notice that this patch determines the name and location of the
>>>> platform relevant files (e.g. src/cpu/ppc/vm/assembler_ppc.cpp or
>>>> src/os_cpu/linux_ppc/vm/globals_linux_ppc.hpp) which will come with
>>>> 0009_linux_ppc_files.patch.
>>>>
>>>> http://cr.openjdk.java.net/~simonis/webrevs/8019922/
>>>>
>>>> Thank you and best regards,
>>>> Volker
>>>>
>>>