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

[Iris Clark]

Hi, David.

The copyright follows the content, so the initial year should not be changed.

iris

-----Original Message-----
From: David Holmes 
Sent: Sunday, July 07, 2013 6:20 PM
To: Iris Clark
Cc: Volker Simonis; ppc-aix-port-dev at openjdk.java.net; HotSpot Open Source Developers
Subject: Re: RFR (S): 8019922 : PPC64 (part 8): Implement Linux/PPC64 support in HotSpot makefiles

Iris,

On 6/07/2013 5:36 AM, Iris Clark wrote:
> For the copyright, please change "2011" to "2013".

What about the 2004 initial year? That doesn't seem right for new files. 
Seems to be due to copying existing files and renaming.

David

> This problem would be caught by various scripts later, but since it's been spotted in a pre-commit, it may as well be handled now.
>
> Thanks,
> iris
>
> -----Original Message-----
> From: David Holmes
> Sent: Thursday, July 04, 2013 10:22 PM
> To: Volker Simonis
> Cc: ppc-aix-port-dev at openjdk.java.net; HotSpot Open Source Developers
> Subject: Re: RFR (S): 8019922 : PPC64 (part 8): Implement Linux/PPC64 
> support in HotSpot makefiles
>
> 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
>
> ---
>
> 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?
>
> ---
>
> 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)
>
> 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 ??
>
>
> 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
>>