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

[Vladimir Kozlov]

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
>>>
>>