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

[Volker Simonis]

On Tue, Jul 9, 2013 at 3:30 AM, David Holmes <david.holmes at oracle.com> wrote:
> Hi Volker,
>
> I'll trim to the remaining issue:
>
>
> On 9/07/2013 1:42 AM, Volker Simonis wrote:
>>
>> On Fri, Jul 5, 2013 at 7:21 AM, David Holmes <david.holmes at oracle.com>
>> wrote:
>>>
>>> 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.
>
>
> So this is broken. If you use HS_ARCH=ppc then you will never use the
>
> ADD_SA_BINARIES/ppc64
>
> definition because it is selected based on HS_ARCH. This suggests you want
> HS_ARCH=ppc64.
>
> However, as you pointed out HS_ARCH is also used in the top-level Makefile:
>
> HS_JNI_ARCH_SRC=$(call
> altsrc-replace,$(HS_COMMON_SRC)/cpu/$(HS_ARCH)/vm/jni_$(HS_ARCH).h)
>
> and I'm assuming that your cpu directory, and arch naming is using ppc here
> not ppc64. In which case you do want HS_ARCH=ppc.
>
> So there is a conflict. Simple solution is to delete
>
> ADD_SA_BINARIES/ppc64
>
> but if you/we were ever to add the 32-bit PPC port and they differed in
> whether SA was supported, then we would have to resolve that somehow. As you
> know we have far too many "arch" variables in the hotspot build system, but
> in this case it isn't clear to me that HS_ARCH should be being used for the
> two situations it presently is.
>

OK, I'll remove ADD_SA_BINARIES/ppc64 and wait with the general fix
until you add the 32-bit PPC port AND only one of the two ports will
exclusivley support SA (whenever that will happen:)

Ready to push now?

Thank you and best regards,
Volker

> Thanks,
> David
> -----
>
>
>>>
>>> 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
>>>>
>>>
>