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

[Volker Simonis]

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