RFR (S): 8019922 : PPC64 (part 8): Implement Linux/PPC64 support in HotSpot makefiles
Volker Simonis
volker.simonis at gmail.com
Wed Jul 10 08:28:13 PDT 2013
Hi Vladimir,
do you want to push this change or should I do it?
Here's the last version with (including David's suggestion of removing
''ADD_SA_BINARIES/ppc64"):
http://cr.openjdk.java.net/~simonis/webrevs/8019922.v3/
Thank you and best regards,
Volker
On Tue, Jul 9, 2013 at 6:24 AM, Vladimir Kozlov
<vladimir.kozlov at oracle.com> wrote:
> 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
>>>>>
>>>>
>
More information about the ppc-aix-port-dev
mailing list