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

[David Holmes]

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.

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