RFR(L): 8161259: Simplify including platform files.
David Holmes
david.holmes at oracle.com
Thu Jul 21 10:00:25 UTC 2016
On 21/07/2016 7:27 PM, Lindenmaier, Goetz wrote:
> Hi David,
>
> Copyright of register_definitions_x86.cpp is already fixed in hs-comp,
> I'll skip it to avoid merges.
This isn't going into hs-comp so I don't know when the two will collide.
I would deal with it anyway - I can just apply the patch without
committing, deal with any merges, and then commit as you.
> I fixed the others. New webrevs, also with Coleens fixes:
> http://cr.openjdk.java.net/~goetz/wr16/8161259-newPfmIncl/webrev.05/
I see the typo was actually much bigger than I thought :) Presumably the
INLCUDE typo caused it to be missed by a global search replace.
> I also did another zero build configured as follows:
> --disable-hotspot-gtest --with-debug-level=fastdebug --with-jvm-variants=zero
> on linuxx86_64.
Thanks.
David
> Best regards,
> Goetz.
>
>> -----Original Message-----
>> From: David Holmes [mailto:david.holmes at oracle.com]
>> Sent: Donnerstag, 21. Juli 2016 05:26
>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Kim Barrett
>> <kim.barrett at oracle.com>
>> Cc: hotspot-compiler-dev at openjdk.java.net; hotspot-runtime-
>> dev at openjdk.java.net
>> Subject: Re: RFR(L): 8161259: Simplify including platform files.
>>
>> On 20/07/2016 8:32 PM, Lindenmaier, Goetz wrote:
>>> Hi
>>>
>>> New webrev: http://cr.openjdk.java.net/~goetz/wr16/8161259-
>> newPfmIncl/webrev.04/
>>>
>>> It re-adds -DTARGET_ARCH_$(HOTSPOT_TARGET_CPU_ARCH) in
>> CompileJvm.gmk
>>> and reverts the change to the aarch define in vmStructs_jvmci.cpp. I
>> documented
>>> what I learned about the platform defines in macros.hpp.
>>
>> Thanks - much appreciated. Other than Coleen's typo (well spotted!) and
>> the lingering _32 the only nit I have left is a handful of copyright
>> updates:
>>
>> src/cpu/x86/vm/register_definitions_x86.cpp:
>> src/share/vm/gc/shared/memset_with_concurrent_readers.hpp:
>> src/share/vm/runtime/semaphore.hpp:
>> src/cpu/ppc/vm/stubRoutines_ppc.hpp:
>> src/cpu/ppc/vm/templateTable_ppc.hpp
>>
>> Then all we need is confirmation that all the open platforms that Oracle
>> doesn't also build (aarch64 and Zero) build and run successfully after
>> this change.
>>
>> I will sponsor this (in case that wasn't clear) but may have to delay it
>> until after we sync up the hs forest with the CPU changes now in
>> jdk9/dev. I will rebase and handle any merging if needed.
>>
>> Thanks,
>> David
>> -----
>>
>>> Best regards,
>>> Goetz.
>>>
>>>> -----Original Message-----
>>>> From: David Holmes [mailto:david.holmes at oracle.com]
>>>> Sent: Mittwoch, 20. Juli 2016 12:05
>>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Kim Barrett
>>>> <kim.barrett at oracle.com>
>>>> Cc: hotspot-compiler-dev at openjdk.java.net; hotspot-runtime-
>>>> dev at openjdk.java.net
>>>> Subject: Re: RFR(L): 8161259: Simplify including platform files.
>>>>
>>>> On 20/07/2016 7:21 PM, Lindenmaier, Goetz wrote:
>>>>> Hi David,
>>>>>
>>>>> OK, to get this through I will add
>>>>> -DTARGET_ARCH_$(HOTSPOT_TARGET_CPU_ARCH)
>>>>> and revert this one and only place it needs to be used in the
>>>> vmStructs_jvmci.cpp.
>>>>
>>>> Thanks.
>>>>
>>>>> For the records, openJdk aarch64 has a C1 port. And yes, such cleanups
>>>> should
>>>>> not be in this change.
>>>>
>>>> I did not know they had a 64-bit C1 - interesting.
>>>>
>>>>>
>>>>> Thanks for doing the closed changes!
>>>>
>>>> You're welcome.
>>>>
>>>> Cheers,
>>>> David
>>>>
>>>>> Best regards,
>>>>> Goetz.
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: David Holmes [mailto:david.holmes at oracle.com]
>>>>>> Sent: Mittwoch, 20. Juli 2016 11:13
>>>>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Kim Barrett
>>>>>> <kim.barrett at oracle.com>
>>>>>> Cc: hotspot-compiler-dev at openjdk.java.net; hotspot-runtime-
>>>>>> dev at openjdk.java.net
>>>>>> Subject: Re: RFR(L): 8161259: Simplify including platform files.
>>>>>>
>>>>>> Hi Goetz,
>>>>>>
>>>>>> On 20/07/2016 6:47 PM, Lindenmaier, Goetz wrote:
>>>>>>> Hi David,
>>>>>>>
>>>>>>> that's great what you are saying and just the design I would expect!
>>>>>>>> We did not want to have to
>>>>>>>> pollute the shared sources with two sets of ifdefs for "64-bit ARM"
>> so
>>>>>>>> we worked with the Open port to ensure that shared code guarded
>> by
>>>>>>>> AARCH64 is suitable for both. We also ensured ARM was used to
>>>> identify
>>>>>>>> word-size agnostic code and we introduced ARM32 in a handful of
>>>> places
>>>>>>>> that needed it. And sometimes we have to be careful and ensure
>> that
>>>>>>>> ifdef chains check AARCH64 before they check ARM.
>>>>>>>
>>>>>>> I think as a consequence the open AARCH port should define ARM,
>> too.
>>>>>>
>>>>>> I would not want to do this and certainly not as part of this change.
>>>>>> If/when the Aarch32 port arrives we may have to revisit this, but not
>>>>>> right now, please.
>>>>>>
>>>>>>> I checked the occurrences and only see three places where this would
>>>> have
>>>>>>> an effect and would have to be fixed somehow:
>>>>>>>
>>>>>>> *** share/vm/jvmci/vmStructs_jvmci.cpp:
>>>>>>> <global>[610] #if defined(AARCH64) && !defined(ARM)
>>>>>>> ==> Would this break the closed port if defined?
>>>>>>
>>>>>> Yes - it refers to specific piece of code in the open aarch64 sources.
>>>>>>
>>>>>>> (This is the only place where TARGET_ARCH_aarch64 was used)
>>>>>>>
>>>>>>> *** share/vm/c1/c1_LIRGenerator.cpp:
>>>>>>> load_item_force[253] #if !defined(ARM) && !defined(E500V2)
>>>>>>> ==> Would this break the open port if defined?
>>>>>>>
>>>>>>> *** share/vm/c1/c1_Runtime1.cpp:
>>>>>>> <global>[1154] #ifdef ARM
>>>>>>> ==> Would this break the open port if defined?
>>>>>>
>>>>>> I don't think the open port has C1 so it would ignore the above files
>>>>>> anyway.
>>>>>>
>>>>>>> All the cases below are pointless.
>>>>>>>
>>>>>>>> So what I'm suggesting is just not getting rid of those defines, but
>>>>>>>> keeping them so they can be used as include guards (or other
>>>> conditional
>>>>>>>> code) when needed, and where the macros are not suitable.
>>>>>>> I think it's quite misleading to have two defines that are 99%
>> equivalent.
>>>>>>> If we really need a special case here, you should define -
>> DARM_CLOSED
>>>>>>> or the like in your closed port. Such a name would make clear what's
>> the
>>>>>>> issue. At least, only your closed port has this problem.
>>>>>>
>>>>>> I really do not want to make any changes to this - ignoring the include
>>>>>> macro changes everything we have is working perfectly fine just the
>> way
>>>>>> the defines are. So I don't want to see this change potentially break
>>>>>> that through incidental changes.
>>>>>>
>>>>>> I do not see having the following is a big issue:
>>>>>>
>>>>>> -DTARGET_ARCH_$(HOTSPOT_TARGET_CPU_ARCH)
>>>>>> -DINCLUDE_SUFFIX_CPU=_$(HOTSPOT_TARGET_CPU_ARCH)
>>>>>>
>>>>>> It allows TARGET_ARCH_aarch64 to mean the open ARMv8 port, and
>>>>>> TARGET_ARCH_arm to mean whatever the owners of that define
>> intend
>>>> it to
>>>>>> mean. It certainly is a lot better than interpreting what the
>>>>>> combinations of AARCH64 and ARM mean. Keeping this removes the
>>>> need to
>>>>>> perform some of the changes as noted above.
>>>>>>
>>>>>> I'm preparing the review of the internal changes we need to
>> accompany
>>>> this.
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>> -----
>>>>>>
>>>>>>>
>>>>>>> Best regards,
>>>>>>> Goetz.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> These should not break the open port if ARM gets defined, or can be
>>>> fixed
>>>>>> easily.
>>>>>>> ---------------------------------------------------------
>>>>>>>
>>>>>>> *** share/vm/c1/c1_LIR.cpp:
>>>>>>> print[1519] #elif defined(ARM)
>>>>>>> ==> Use ARM32 as after AARCH64 in if-cascade.
>>>>>>>
>>>>>>> *** os/linux/vm/os_linux.cpp:
>>>>>>> dll_load[1796] #elif (defined ARM)
>>>>>>> get_summary_cpu_info[2273] #elif defined(ARM)
>>>>>>> ==> Use ARM32 as after AARCH64 in if-cascade.
>>>>>>>
>>>>>>> *** share/vm/opto/matcher.cpp:
>>>>>>> init_first_stack_mask[558] #ifdef ARM
>>>>>>> ==> Should be ARM32 (Is under !LP64).
>>>>>>>
>>>>>>> *** share/vm/c1/c1_LIR.cpp:
>>>>>>> validate_type[212] ARM_ONLY(|| kindfield == cpu_register)
>>>>>>> validate_type[219] ARM_ONLY(|| kindfield == cpu_register)
>>>>>>> ==> Just an assertion. Will just relax the check if defined in open
>>>> AARCH64.
>>>>>>> But maybe should be guarded by __SOFTFP__.
>>>>>>> <global>[70] #if defined(ARM) || defined(AARCH64) ||
>>>>>> defined(PPC64)
>>>>>>> ==> Fine: ||
>>>>>>>
>>>>>>> *** share/vm/c1/c1_LIR.hpp:
>>>>>>> <global>[447] #if defined(SPARC) || defined(ARM) ||
>>>>>> defined(PPC) || defined(AARCH64)
>>>>>>> <global>[537] #if defined(X86) || defined(ARM) ||
>>>>>> defined(AARCH64)
>>>>>>> ==> Fine: ||
>>>>>>>
>>>>>>> *** share/vm/interpreter/interpreterRuntime.hpp:
>>>>>>> defined[162] #if defined(IA32) || defined(AMD64) ||
>>>>>> defined(ARM)
>>>>>>> *** share/vm/interpreter/interpreterRuntime.cpp:
>>>>>>> <global>[1358] #if defined(IA32) || defined(AMD64) ||
>>>>>> defined(ARM)
>>>>>>> ==> Just defines a method which would be unused, should be fine.
>>>>>>>
>>>>>>>
>>>>>>> These are in code not used in the open AARCH64 port:
>>>>>>> --------------------------------------------------
>>>>>>>
>>>>>>> *** os/bsd/vm/os_bsd.cpp:
>>>>>>> <global>[215] #elif defined(ARM)
>>>>>>>
>>>>>>> *** os_cpu/bsd_zero/vm/atomic_bsd_zero.inline.hpp:
>>>>>>> <global>[102] #ifdef ARM
>>>>>>>
>>>>>>> *** os_cpu/bsd_zero/vm/orderAccess_bsd_zero.inline.hpp:
>>>>>>> <global>[31] #ifdef ARM
>>>>>>>
>>>>>>> *** os_cpu/linux_zero/vm/atomic_linux_zero.inline.hpp:
>>>>>>> <global>[102] #ifdef ARM
>>>>>>>
>>>>>>> *** os_cpu/linux_zero/vm/orderAccess_linux_zero.inline.hpp:
>>>>>>> <global>[31] #ifdef ARM
>>>>>>>
>>>>>>> *** share/vm/utilities/macros.hpp:
>>>>>>> <global>[434] #ifdef ARM
>>>>>>>
>>>>>>> *** os/bsd/vm/os_bsd.cpp:
>>>>>>> dll_load[1508] #elif (defined ARM)
>>>>>>> dll_load[1524] IA32, AMD64, IA64, __sparc, __powerpc__,
>> ARM,
>>>>>> S390, ALPHA, MIPS, MIPSEL, PARISC, M68K
>>>>>>>
>>>>>>> *** os/solaris/vm/os_solaris.cpp:
>>>>>>> dll_load[1725] #elif (defined ARM)
>>>>>>> dll_load[1729] IA32, AMD64, IA64, __sparc, __powerpc__,
>> ARM,
>>>>>> ARM
>>>>>>>
>>>>>>> *** os_cpu/bsd_zero/vm/atomic_bsd_zero.inline.hpp:
>>>>>>> store[164] #if !defined(ARM) && !defined(M68K)
>>>>>>> store_ptr[171] #if !defined(ARM) && !defined(M68K)
>>>>>>> add[178] #ifdef ARM
>>>>>>> add_ptr[190] #ifdef ARM
>>>>>>> xchg[230] #ifdef ARM
>>>>>>> xchg_ptr[253] #ifdef ARM
>>>>>>> cmpxchg[275] #ifdef ARM
>>>>>>> cmpxchg_ptr[298] #ifdef ARM
>>>>>>>
>>>>>>> *** os_cpu/linux_zero/vm/atomic_linux_zero.inline.hpp:
>>>>>>> add[172] #ifdef ARM
>>>>>>> add_ptr[184] #ifdef ARM
>>>>>>> xchg[224] #ifdef ARM
>>>>>>> xchg_ptr[247] #ifdef ARM
>>>>>>> cmpxchg[269] #ifdef ARM
>>>>>>> cmpxchg_ptr[292] #ifdef ARM
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: David Holmes [mailto:david.holmes at oracle.com]
>>>>>>>> Sent: Dienstag, 19. Juli 2016 23:59
>>>>>>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Kim Barrett
>>>>>>>> <kim.barrett at oracle.com>
>>>>>>>> Cc: hotspot-compiler-dev at openjdk.java.net; hotspot-runtime-
>>>>>>>> dev at openjdk.java.net
>>>>>>>> Subject: Re: RFR(L): 8161259: Simplify including platform files.
>>>>>>>>
>>>>>>>> Hi Goetz,
>>>>>>>>
>>>>>>>> On 19/07/2016 10:12 PM, Lindenmaier, Goetz wrote:
>>>>>>>>> Hi David,
>>>>>>>>>
>>>>>>>>> we also have "closed ports", currently HPUX, ia64 and s390.
>>>>>>>>> (Parisc is gone, puh!).
>>>>>>>>> We basically patch all the shared changes onto the sources after
>>>>>>>>> getting them via our licensee channel.
>>>>>>>>> I think you should fix your closed port not to re-use the names of
>> the
>>>>>>>>> main openJdk platforms!
>>>>>>>>
>>>>>>>> Nobody "owns" a define of AARCH64 or ARM. We did not want to
>> have
>>>> to
>>>>>>>> pollute the shared sources with two sets of ifdefs for "64-bit ARM"
>> so
>>>>>>>> we worked with the Open port to ensure that shared code guarded
>> by
>>>>>>>> AARCH64 is suitable for both. We also ensured ARM was used to
>>>> identify
>>>>>>>> word-size agnostic code and we introduced ARM32 in a handful of
>>>> places
>>>>>>>> that needed it. And sometimes we have to be careful and ensure
>> that
>>>>>>>> ifdef chains check AARCH64 before they check ARM.
>>>>>>>>
>>>>>>>> This has all been working quite nicely, as the include guards used, for
>>>>>>>> example, TARGET_ARCH_AARCH64 and TARGET_ARCH_ARM - which
>>>> are
>>>>>>>> never
>>>>>>>> defined at the same time (derived from
>>>> HOTSPOT_TARGET_CPU_ARCH).
>>>>>> But
>>>>>>>> the
>>>>>>>> current changes remove those unique defines and, before the
>>>> HEADER_H
>>>>>>>> forms were introduced, tried to use simple AARCH64 and ARM as
>>>> include
>>>>>>>> guards, and that doesn't work as they are not mutually exclusive.
>>>>>>>>
>>>>>>>> So what I'm suggesting is just not getting rid of those defines, but
>>>>>>>> keeping them so they can be used as include guards (or other
>>>> conditional
>>>>>>>> code) when needed, and where the macros are not suitable.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> David
>>>>>>>> -----
>>>>>>>>
>>>>>>>>> I have no idea what hardware is addressed by your closed ports,
>>>>>>>>> nor how you merge sources.
>>>>>>>>> Is there also a port that sets
>>>>>>>>> -DTARGET_ARCH_arm
>>>>>>>>> -DARM
>>>>>>>>> -DARM32
>>>>>>>>> ?
>>>>>>>>>
>>>>>>>>> To fix this either you define
>>>>>>>>> #if defined(ARM) && defined(_LP64)
>>>>>>>>> #define ARM64
>>>>>>>>> #endif
>>>>>>>>> in macros.hpp or you set -DARM64 on the command line.
>>>>>>>>> Then you replace all
>>>>>>>>> #ifdef AARCH64
>>>>>>>>> by
>>>>>>>>> #if defined(AARCH64) || defined(ARM64)
>>>>>>>>> or maybe it suffices altogether f you replace
>>>>>>>>> #ifdef AARCH64
>>>>>>>>> by
>>>>>>>>> #if defined(AARCH64) || defined(ARM)
>>>>>>>>>
>>>>>>>>> For ppc, when we did the port we knew (and that's all we knew)
>>>>>>>>> that you have a 32-bit port. Therefore we set up these macros
>>>>>>>>> as on x86, where there is one for the arch (X86) and two for
>>>> LP64/!LP64
>>>>>>>>> (IA32, AMD64). This allowed to separate code for the closed port
>>>>>>>>> (guarded by PPC32), the open port (PPC64) and shared for both
>>>> (PPC).
>>>>>>>>>
>>>>>>>>> But I don't think it is necessary to do any further changes now,
>>>> assuming
>>>>>>>>> my change works for you as I adapted it. So we're all set I guess!
>>>>>>>>>
>>>>>>>>> Best regards,
>>>>>>>>> Goetz.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: David Holmes [mailto:david.holmes at oracle.com]
>>>>>>>>>> Sent: Tuesday, July 19, 2016 1:23 PM
>>>>>>>>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Kim
>> Barrett
>>>>>>>>>> <kim.barrett at oracle.com>
>>>>>>>>>> Cc: hotspot-compiler-dev at openjdk.java.net; hotspot-runtime-
>>>>>>>>>> dev at openjdk.java.net
>>>>>>>>>> Subject: Re: RFR(L): 8161259: Simplify including platform files.
>>>>>>>>>>
>>>>>>>>>> On 19/07/2016 7:08 PM, Lindenmaier, Goetz wrote:
>>>>>>>>>>> Hi David,
>>>>>>>>>>>
>>>>>>>>>>>> I'm still uneasy that TARGET_ARCH_xxx is now just xxx. This
>> kind
>>>> of
>>>>>>>>>>>> workaround is obscure - you have to know that the Open
>> Aarch64
>>>>>> port
>>>>>>>>>>>> defines AARCH64 but not ARM and so that code is for the Open
>>>> port
>>>>>>>> use
>>>>>>>>>>>> only. There's no issue for the OS defines, but I wonder - just
>>>>>> something
>>>>>>>>>>>> to thing about - if TARGET_ARCH_xxx should be restored?
>>>>>>>>>>>
>>>>>>>>>>> Well, I think TARGET_ARCH_xxx always was xxx.
>>>>>>>>>>> And I'm uneasy that it is no more. How do you handle that? You
>>>> have
>>>>>> to
>>>>>>>>>>> check every AARCH change by RedHat against your closed port?
>>>>>>>>>>
>>>>>>>>>> The sources for the two ports are distinct so the only place we
>> have
>>>> to
>>>>>>>>>> have a convention is in shared code that has CPU specific stuff and
>> in
>>>>>>>>>> the build files.
>>>>>>>>>>
>>>>>>>>>> The open Aarch64 port sets (among others):
>>>>>>>>>> -DTARGET_ARCH_aarch64
>>>>>>>>>> -DAARCH64
>>>>>>>>>>
>>>>>>>>>> the closed port sets
>>>>>>>>>>
>>>>>>>>>> -DTARGET_ARCH_arm
>>>>>>>>>> -DARM
>>>>>>>>>> -DAARCH64
>>>>>>>>>>
>>>>>>>>>> so it is the value of TARGET_ARCH_xxx that distinguishes them.
>>>>>>>> Whenever
>>>>>>>>>> you saw TARGET_ARCH_arm in open shared code it is/was
>> referring
>>>> to
>>>>>>>> our
>>>>>>>>>> closed port; and TARGET_ARCH_aarch64 refers to the open
>> Aarch64
>>>>>> port.
>>>>>>>>>>
>>>>>>>>>> Of course TARGET_OS_ARCH_linux_xxx is in the same position.
>>>>>>>>>>
>>>>>>>>>> We need to keep a clear distinction. Using the combination of
>>>> AARCH64
>>>>>>>>>> and ARM is not so clear.
>>>>>>>>>>
>>>>>>>>>> You could easily have similar issues with other port groups - eg
>> ppc64
>>>>>>>>>> vs ppc32 vs ppcle.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> David
>>>>>>>>>>
>>>>>>>>>>> I don't know about the closed stuff, but aarch came up recently,
>>>> and
>>>>>>>>>>> before that it sure was equivalent. And it still is the case for
>>>> openJDK.
>>>>>>>>>>>
>>>>>>>>>>> Below output is grepped out of the make/<os>/platform_<cpu>
>>>> files
>>>>>> in
>>>>>>>>>>> jdk8/hotspot, and none of the cpu/arch names are defined
>> twice.
>>>>>>>>>>> Zero is an exception I guess, as it's no real cpu/arch.
>>>>>>>>>>>
>>>>>>>>>>> Best regards,
>>>>>>>>>>> Goetz.
>>>>>>>>>>>
>>>>>>>>>>> sysdefs = -DAIX -DPPC64
>>>>>>>>>>> sysdefs = -D_ALLBSD_SOURCE -D_GNU_SOURCE -DAMD64
>>>>>>>>>>> sysdefs = -D_ALLBSD_SOURCE -DSPARC_WORKS -
>> D_GNU_SOURCE
>>>> -
>>>>>>>>>> DAMD64
>>>>>>>>>>> sysdefs = -D_ALLBSD_SOURCE -D_GNU_SOURCE -DIA32
>>>>>>>>>>> sysdefs = -D_ALLBSD_SOURCE -DSPARC_WORKS -
>> D_GNU_SOURCE
>>>> -
>>>>>>>> DIA32
>>>>>>>>>>> sysdefs = -D_ALLBSD_SOURCE -D_GNU_SOURCE -DIA64 -
>>>> DCC_INTERP
>>>>>>>>>>> sysdefs = -D_ALLBSD_SOURCE -D_GNU_SOURCE -DSPARC
>>>>>>>>>>> sysdefs = -D_ALLBSD_SOURCE -D_GNU_SOURCE -DSPARC
>>>>>>>>>>> sysdefs = -D_ALLBSD_SOURCE -D_GNU_SOURCE -DCC_INTERP -
>>>>>> DZERO -
>>>>>>>>>> D at ZERO_ARCHDEF@ -DZERO_LIBARCH=\"@ZERO_LIBARCH@\"
>>>>>>>>>>> sysdefs = -DLINUX -D_GNU_SOURCE -DAMD64
>>>>>>>>>>> sysdefs = -DLINUX -DSPARC_WORKS -D_GNU_SOURCE -
>> DAMD64
>>>>>>>>>>> sysdefs = -DLINUX -D_GNU_SOURCE -DIA32
>>>>>>>>>>> sysdefs = -DLINUX -DSPARC_WORKS -D_GNU_SOURCE -DIA32
>>>>>>>>>>> sysdefs = -DLINUX -D_GNU_SOURCE -DIA64 -DCC_INTERP
>>>>>>>>>>> sysdefs = -DLINUX -D_GNU_SOURCE -DPPC64
>>>>>>>>>>> sysdefs = -DLINUX -D_GNU_SOURCE -DSPARC
>>>>>>>>>>> sysdefs = -DLINUX -D_GNU_SOURCE -DSPARC
>>>>>>>>>>> sysdefs = -DLINUX -D_GNU_SOURCE -DCC_INTERP -DZERO -
>>>>>>>>>> D at ZERO_ARCHDEF@ -DZERO_LIBARCH=\"@ZERO_LIBARCH@\"
>>>>>>>>>>> sysdefs = -DSOLARIS -DSPARC_WORKS -DAMD64
>>>>>>>>>>> sysdefs = -DSOLARIS -D_GNU_SOURCE -DAMD64
>>>>>>>>>>> sysdefs = -DSOLARIS -DSPARC_WORKS -DIA32
>>>>>>>>>>> sysdefs = -DSOLARIS -D_GNU_SOURCE -DIA32
>>>>>>>>>>> sysdefs = -DSOLARIS -DSPARC_WORKS -DSPARC
>>>>>>>>>>> sysdefs = -DSOLARIS -D_GNU_SOURCE -DSPARC
>>>>>>>>>>> sysdefs = -DSOLARIS -DSPARC_WORKS -DSPARC
>>>>>>>>>>> sysdefs = -DSOLARIS -D_GNU_SOURCE -DSPARC
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>> From: David Holmes [mailto:david.holmes at oracle.com]
>>>>>>>>>>>> Sent: Tuesday, July 19, 2016 10:47 AM
>>>>>>>>>>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Kim
>>>> Barrett
>>>>>>>>>>>> <kim.barrett at oracle.com>
>>>>>>>>>>>> Cc: hotspot-compiler-dev at openjdk.java.net; hotspot-runtime-
>>>>>>>>>>>> dev at openjdk.java.net
>>>>>>>>>>>> Subject: Re: RFR(L): 8161259: Simplify including platform files.
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks Goetz. Will get back to you asap if there are any further
>>>>>> issues,
>>>>>>>>>>>> but the incremental changes look okay.
>>>>>>>>>>>>
>>>>>>>>>>>> On 19/07/2016 5:47 PM, Lindenmaier, Goetz wrote:
>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>
>>>>>>>>>>>>> I added macros for C headers:
>>>>>>>>>>>>> CPU_HEADER_H() etc.
>>>>>>>>>>>>> and fixed the other issues mentioned by David and Coleen in
>> this
>>>>>> new
>>>>>>>>>>>> webrev:
>>>>>>>>>>>>> http://cr.openjdk.java.net/~goetz/wr16/8161259-
>>>>>>>>>> newPfmIncl/webrev.03/
>>>>>>>>>>>>>
>>>>>>>>>>>>> I also added comments that AARCH64 and ARM are defined
>>>>>>>>>>>>> at the same time.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Further I edited vmStructs_jvmci.cpp to
>>>>>>>>>>>>> #if defined(AARCH64) && !defined(ARM)
>>>>>>>>>>>>
>>>>>>>>>>>> I'm still uneasy that TARGET_ARCH_xxx is now just xxx. This
>> kind
>>>> of
>>>>>>>>>>>> workaround is obscure - you have to know that the Open
>> Aarch64
>>>>>> port
>>>>>>>>>>>> defines AARCH64 but not ARM and so that code is for the Open
>>>> port
>>>>>>>> use
>>>>>>>>>>>> only. There's no issue for the OS defines, but I wonder - just
>>>>>> something
>>>>>>>>>>>> to thing about - if TARGET_ARCH_xxx should be restored?
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> David
>>>>>>>>>>>>
>>>>>>>>>>>>> My incremental changes are in this patch:
>>>>>>>>>>>>> http://cr.openjdk.java.net/~goetz/wr16/8161259-
>>>>>>>>>>>> newPfmIncl/webrev.03/incremental_changes.patch
>>>>>>>>>>>>>
>>>>>>>>>>>>> Best regards,
>>>>>>>>>>>>> Goetz.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>>>> From: David Holmes [mailto:david.holmes at oracle.com]
>>>>>>>>>>>>>> Sent: Tuesday, July 19, 2016 3:37 AM
>>>>>>>>>>>>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Kim
>>>>>> Barrett
>>>>>>>>>>>>>> <kim.barrett at oracle.com>
>>>>>>>>>>>>>> Cc: hotspot-compiler-dev at openjdk.java.net; hotspot-
>> runtime-
>>>>>>>>>>>>>> dev at openjdk.java.net
>>>>>>>>>>>>>> Subject: Re: RFR(L): 8161259: Simplify including platform files.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi Goetz,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 18/07/2016 10:23 PM, David Holmes wrote:
>>>>>>>>>>>>>>> On 18/07/2016 10:18 PM, Lindenmaier, Goetz wrote:
>>>>>>>>>>>>>>>> Hi David,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I'll introduce CPU_HEADER_H, but actually this should be
>>>> fixed
>>>>>>>>>>>>>>>> in the closed code.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Bear with me, I'm trying to figure out how to do just that. :)
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The use of HOTSPOT_TARGET_CPU_DEFINE as the value
>> for
>>>> the
>>>>>>>> ifdef
>>>>>>>>>> is
>>>>>>>>>>>> not
>>>>>>>>>>>>>>> going to work for our closed ports, unless I change that
>> value
>>>> to
>>>>>>>>>> match
>>>>>>>>>>>>>>> the open naming scheme. But that in turn will lead to other
>>>>>>>> problems
>>>>>>>>>> as
>>>>>>>>>>>>>>> we also need that value the way it is currently defined.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I'll tackle this again in the morning when I'm fresher.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Sorry but this really does need the CPU_HEADER_H macro.
>> In
>>>>>>>> general
>>>>>>>>>> you
>>>>>>>>>>>>>> can't just replace:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> #ifdef TARGET_ARCH_abc
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> with
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> #ifdef abc
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> because the "abc"'s may not be mutually exclusive. In our
>> case
>>>>>> ARM
>>>>>>>>>> and
>>>>>>>>>>>>>> AARCH64 are both defined and are both needed. In contrast
>>>>>>>>>>>>>> TARGET_ARCH_abc is uniquely defined as "abc" is chosen to
>>>>>>>> represent
>>>>>>>>>> the
>>>>>>>>>>>>>> architecture in this context.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> David
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>> David
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Best regards,
>>>>>>>>>>>>>>>> Goetz.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>>>>>>> From: David Holmes [mailto:david.holmes at oracle.com]
>>>>>>>>>>>>>>>>> Sent: Montag, 18. Juli 2016 14:13
>>>>>>>>>>>>>>>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>;
>>>> Kim
>>>>>>>> Barrett
>>>>>>>>>>>>>>>>> <kim.barrett at oracle.com>
>>>>>>>>>>>>>>>>> Cc: hotspot-compiler-dev at openjdk.java.net; hotspot-
>>>>>> runtime-
>>>>>>>>>>>>>>>>> dev at openjdk.java.net
>>>>>>>>>>>>>>>>> Subject: Re: RFR(L): 8161259: Simplify including platform
>>>> files.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Hi Goetz,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On 18/07/2016 8:21 PM, Lindenmaier, Goetz wrote:
>>>>>>>>>>>>>>>>>> Hi David,
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> src/share/vm/prims/jni_md.h
>>>>>>>>>>>>>>>>>>>>> src/share/vm/prims/jvm.h
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> // Can not use CPU_HEADER() macro, as it appends
>>>> .hpp
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> can we not define a second form, eg
>> CPU_HEADER_H,
>>>>>> that
>>>>>>>>>>>>>> appends.h
>>>>>>>>>>>>>>>>>>>>> instead?
>>>>>>>>>>>>>>>>>>>> I'll need one for OS, one for CPU, and each will be
>> used
>>>>>> only
>>>>>>>>>> once.
>>>>>>>>>>>>>>>>>>>> So I figured not to do it. But probably I should do it
>> to
>>>> get
>>>>>> is
>>>>>>>>>>>>>>>>>>>> similar everywhere.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> We actually need this as the simple arch names need
>> not
>>>>>> be
>>>>>>>>>>>> mutually
>>>>>>>>>>>>>>>>>>> exclusive - eg arm and aarch64. Otherwise this needs
>> to
>>>> be
>>>>>> an
>>>>>>>> if-
>>>>>>>>>>>> else
>>>>>>>>>>>>>>>>>>> construct in the correct order. ;-)
>>>>>>>>>>>>>>>>>> They should be mutually exclusive, as they are set in
>>>>>>>>>>>> CompileJvm.gmk
>>>>>>>>>>>>>>>>>> in the same statement as the INCLUDE_SUFFIX_CPU:
>>>>>>>>>>>>>>>>>> -D$(HOTSPOT_TARGET_CPU_DEFINE)
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Unfortunately we also have -DARM coming in from the
>>>> closed
>>>>>>>> part
>>>>>>>>>> of
>>>>>>>>>>>>>>>>> spec.gmk as we don't use AARCH64 for our port. So we
>> get
>>>>>> both
>>>>>>>>>>>> defined
>>>>>>>>>>>>>>>>> and try to include two platform files.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Not sure how to try and resolve this yet. Trying to
>>>> understand
>>>>>>>> what
>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>> role of HOTSPOT_TARGET_CPU_DEFINE is intended to
>> be,
>>>> as
>>>>>> we
>>>>>>>>>> are
>>>>>>>>>>>> not
>>>>>>>>>>>>>>>>> overriding it for our port - which I think is the current
>>>> problem,
>>>>>>>> but
>>>>>>>>>>>>>>>>> changing it may break something else.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>> David
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Best regards,
>>>>>>>>>>>>>>>>>> Goetz
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>>>>>>>>> From: David Holmes
>> [mailto:david.holmes at oracle.com]
>>>>>>>>>>>>>>>>>>> Sent: Montag, 18. Juli 2016 11:06
>>>>>>>>>>>>>>>>>>> To: Lindenmaier, Goetz
>> <goetz.lindenmaier at sap.com>;
>>>>>> Kim
>>>>>>>>>> Barrett
>>>>>>>>>>>>>>>>>>> <kim.barrett at oracle.com>
>>>>>>>>>>>>>>>>>>> Cc: hotspot-compiler-dev at openjdk.java.net;
>> hotspot-
>>>>>>>> runtime-
>>>>>>>>>>>>>>>>>>> dev at openjdk.java.net
>>>>>>>>>>>>>>>>>>> Subject: Re: RFR(L): 8161259: Simplify including
>> platform
>>>>>> files.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> On 18/07/2016 5:15 PM, Lindenmaier, Goetz wrote:
>>>>>>>>>>>>>>>>>>>> Hi David,
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> thanks for looking at all these files in more detail!
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>>>>>>>>>>> From: David Holmes
>>>> [mailto:david.holmes at oracle.com]
>>>>>>>>>>>>>>>>>>>>> Sent: Montag, 18. Juli 2016 08:03
>>>>>>>>>>>>>>>>>>>>> To: Lindenmaier, Goetz
>>>> <goetz.lindenmaier at sap.com>;
>>>>>>>> Kim
>>>>>>>>>>>> Barrett
>>>>>>>>>>>>>>>>>>>>> <kim.barrett at oracle.com>
>>>>>>>>>>>>>>>>>>>>> Cc: hotspot-compiler-dev at openjdk.java.net;
>>>> hotspot-
>>>>>>>> runtime-
>>>>>>>>>>>>>>>>>>>>> dev at openjdk.java.net
>>>>>>>>>>>>>>>>>>>>> Subject: Re: RFR(L): 8161259: Simplify including
>>>> platform
>>>>>>>> files.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Hi Goetz,
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Again thanks for tackling this!
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> On 15/07/2016 10:17 PM, Lindenmaier, Goetz
>> wrote:
>>>>>>>>>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> I made a new webrev:
>>>>>>>>>>>>>>>>>>>>>> - '_' is added in makefile
>>>>>>>>>>>>>>>>>>>>>> - uses Kim's macros
>>>>>>>>>>>>>>>>>>>>>> - macros are capitalized
>>>>>>>>>>>>>>>>>>>>>> - more comments in macros.hpp
>>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~goetz/wr16/8161259-
>>>>>>>>>>>>>>>>>>> newPfmIncl/webrev.02/
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Generally looks okay, a couple of clarifications and
>>>>>>>> comments
>>>>>>>>>>>>>> below.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> make/gensrc/GensrcAdlc.gmk
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> So the change from HOTSPOT_TARGET_CPU to
>>>>>>>>>>>>>>>>>>>>> HOTSPOT_TARGET_CPU_ARCH only
>>>>>>>>>>>>>>>>>>>>> affects the generated files right?
>>>>>>>>>>>>>>>>>>>> Yes.
>>>>>>>>>>>>>>>>>>>>> For our closed ports we still have
>>>>>>>>>>>>>>>>>>>>> the _32/_64 source files in a common arch
>> directory,
>>>> but
>>>>>> I
>>>>>>>>>> don't
>>>>>>>>>>>>>>>>>>>>> think
>>>>>>>>>>>>>>>>>>>>> that needs to be reflected in the generated files.
>>>> (Sorry I
>>>>>>>>>>>>>>>>>>>>> haven't had
>>>>>>>>>>>>>>>>>>>>> the time yet to apply this patch and see what needs
>> to
>>>>>> be
>>>>>>>>>>>> changed
>>>>>>>>>>>>>> on
>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>>> closed side - but will start that once I send this :).)
>>>>>>>>>>>>>>>>>>>> If I run plain configure, it generates a directory that
>> has
>>>>>> the
>>>>>>>>>>>>>>>>>>>> word size
>>>>>>>>>>>>>>>>>>>> in it's name:
>>>>>>>>>>>>>>>>>>>> linux-x86_64-normal-server-release
>>>>>>>>>>>>>>>>>>>> If I configure --with-target-bits=32, it builds to
>>>>>>>>>>>>>>>>>>>> linux-x86-normal-server-release,
>>>>>>>>>>>>>>>>>>>> so I figured this should be fine.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Yes generated files are fine.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> make/lib/CompileJvm.gmk
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Hmm but this change assumes no more _32/_64
>>>> header
>>>>>>>> files if
>>>>>>>>>> I
>>>>>>>>>>>>>>>>>>>>> read it
>>>>>>>>>>>>>>>>>>>>> right ?? So I'll need a common file that dispatches
>>>>>> internally
>>>>>>>> for
>>>>>>>>>>>>>>>>>>>>> 32-bit and 64-bit.
>>>>>>>>>>>>>>>>>>>> Yes.
>>>>>>>>>>>>>>>>>>>> There is a single file in cpu/x86 where this did not
>> hold:
>>>>>>>>>>>>>>>>>>>> stubRoutines
>>>>>>>>>>>>>>>>>>>> But this was rather small. And there anyways was a
>>>>>> common
>>>>>>>> file
>>>>>>>>>>>>>>>>>>>> that was included in both.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> src/os/posix/vm/os_posix.cpp
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> I'm wondering if we can use a similar trick to avoid
>> this
>>>>>> kind
>>>>>>>> of
>>>>>>>>>>>>>>>>>>>>> switch(OS) statement:
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> #ifdef TARGET_OS_FAMILY_linux
>>>>>>>>>>>>>>>>>>>>> Linux::ucontext_set_pc(ctx, pc);
>>>>>>>>>>>>>>>>>>>>> #elif defined(TARGET_OS_FAMILY_solaris)
>>>>>>>>>>>>>>>>>>>>> Solaris::ucontext_set_pc(ctx, pc);
>>>>>>>>>>>>>>>>>>>>> ...
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> ie something like:
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> SUB(TARGET_OS)::ucontext_set_pc(ctx, pc);
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> ? :)
>>>>>>>>>>>>>>>>>>>> Hmm, I had to add the '_' to the string in
>> TARGET_SO...
>>>>>>>>>>>>>>>>>>>> Actually I think the implementation should be moved
>> to
>>>>>>>>>>>>>>>>>>>> os_linux.cpp/os_bsd.cpp etc.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> src/share/vm/code/nmethod.cpp
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Not clear why the platform include can simply be
>>>> elided
>>>>>>>> here ??
>>>>>>>>>>>>>>>>>>>> It already includes code/nativeInst.hpp, which is the
>>>>>>>> umbrella
>>>>>>>>>>>> header
>>>>>>>>>>>>>>>>>>>> for all the platform files.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> src/share/vm/jvmci/jvmciRuntime.cpp
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Shouldn't:
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> ! #endif // !LP64
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> be:
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> ! #endif // LP64
>>>>>>>>>>>>>>>>>>>> Well, it ends the #else part ... but fixed anyways.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> src/share/vm/prims/jni_md.h
>>>>>>>>>>>>>>>>>>>>> src/share/vm/prims/jvm.h
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> // Can not use CPU_HEADER() macro, as it appends
>>>> .hpp
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> can we not define a second form, eg
>> CPU_HEADER_H,
>>>>>> that
>>>>>>>>>>>>>> appends.h
>>>>>>>>>>>>>>>>>>>>> instead?
>>>>>>>>>>>>>>>>>>>> I'll need one for OS, one for CPU, and each will be
>> used
>>>>>> only
>>>>>>>>>> once.
>>>>>>>>>>>>>>>>>>>> So I figured not to do it. But probably I should do it
>> to
>>>> get
>>>>>> is
>>>>>>>>>>>>>>>>>>>> similar everywhere.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> We actually need this as the simple arch names need
>> not
>>>>>> be
>>>>>>>>>>>> mutually
>>>>>>>>>>>>>>>>>>> exclusive - eg arm and aarch64. Otherwise this needs
>> to
>>>> be
>>>>>> an
>>>>>>>> if-
>>>>>>>>>>>> else
>>>>>>>>>>>>>>>>>>> construct in the correct order. ;-)
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>> David
>>>>>>>>>>>>>>>>>>> -----
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> src/share/vm/runtime/os.hpp
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Should we convert setjmp.h include to:
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> #ifndef _WINDOWS
>>>>>>>>>>>>>>>>>>>>> #include <setjmp.h>
>>>>>>>>>>>>>>>>>>>>> #endif
>>>>>>>>>>>>>>>>>>>>> #ifdef __APPLE__
>>>>>>>>>>>>>>>>>>>>> ...
>>>>>>>>>>>>>>>>>>>>> #endif
>>>>>>>>>>>>>>>>>>>>> ?
>>>>>>>>>>>>>>>>>>>> Fixed.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> BTW: why is it _WINDOWS instead of WINDOWS? Is
>>>> that
>>>>>>>>>> coming
>>>>>>>>>>>>>> from
>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>>> compiler itself?
>>>>>>>>>>>>>>>>>>>> I wondered about that, too. Therefore I defined
>>>>>> WINDOWS
>>>>>>>> in
>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>> prototype webrev. Then I saw that our build is
>> defining
>>>> -
>>>>>>>>>>>>>> D_WINDOWS
>>>>>>>>>>>>>>>>>>> *and* -DWIN32,
>>>>>>>>>>>>>>>>>>>> and removed it again using _WINDOWS instead.
>>>>>>>>>>>>>>>>>>>> Eventually both should be replaced by WINDOWS,
>> but
>>>> not
>>>>>> in
>>>>>>>> this
>>>>>>>>>>>>>>>>>>>> change.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> src/os_cpu/aix_ppc/vm/bytes_aix_ppc.inline.hpp
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Don't understand the point of this:
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> 28 #if defined(VM_LITTLE_ENDIAN)
>>>>>>>>>>>>>>>>>>>>> 29 // Aix is not littel endian.
>>>>>>>>>>>>>>>>>>>>> 30 #endif // VM_LITTLE_ENDIAN
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> also typo: littel
>>>>>>>>>>>>>>>>>>>> I just copied the linux_ppc code and removed the
>>>> unused
>>>>>>>>>>>>>>>>> implementation.
>>>>>>>>>>>>>>>>>>>> I wanted to document why there is this empty file.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Best regards,
>>>>>>>>>>>>>>>>>>>> Goetz.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>> David
>>>>>>>>>>>>>>>>>>>>> -----
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> Best regards,
>>>>>>>>>>>>>>>>>>>>>> Goetz.
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>>>>>>>>>>>>> From: David Holmes
>>>>>> [mailto:david.holmes at oracle.com]
>>>>>>>>>>>>>>>>>>>>>>> Sent: Freitag, 15. Juli 2016 11:11
>>>>>>>>>>>>>>>>>>>>>>> To: Lindenmaier, Goetz
>>>>>> <goetz.lindenmaier at sap.com>;
>>>>>>>> Kim
>>>>>>>>>>>>>> Barrett
>>>>>>>>>>>>>>>>>>>>>>> <kim.barrett at oracle.com>
>>>>>>>>>>>>>>>>>>>>>>> Cc: hotspot-compiler-dev at openjdk.java.net;
>>>>>> hotspot-
>>>>>>>>>> runtime-
>>>>>>>>>>>>>>>>>>>>>>> dev at openjdk.java.net
>>>>>>>>>>>>>>>>>>>>>>> Subject: Re: RFR(L): 8161259: Simplify including
>>>>>> platform
>>>>>>>>>> files.
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> On 15/07/2016 5:30 PM, Lindenmaier, Goetz
>> wrote:
>>>>>>>>>>>>>>>>>>>>>>>> HI Kim,
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> thanks for this version of the macros, it's
>> working
>>>> on
>>>>>> all
>>>>>>>> the
>>>>>>>>>>>>>>>>> platforms
>>>>>>>>>>>>>>>>>>>>>>>> I can build.
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> Actually, I would prefer having the '_' in the
>>>> macros
>>>>>> and
>>>>>>>>>> not
>>>>>>>>>>>>>>>>>>>>>>>> on the
>>>>>>>>>>>>>>>>>>>>>>>> command line. This way, parts of the name
>>>>>>>> construction
>>>>>>>>>> are
>>>>>>>>>>>> in
>>>>>>>>>>>>>>>>>>>>>>>> CompileJvm.gmk, other parts are in
>> macros.hpp.
>>>> But
>>>>>>>>>>>>>> constructing
>>>>>>>>>>>>>>>>>>>>>>>> the search path is also in the makefile, and uses
>>>> the
>>>>>>>> very
>>>>>>>>>>>> same
>>>>>>>>>>>>>>>>> string
>>>>>>>>>>>>>>>>>>>>>>>> as the file suffixes. (Without '_', one could
>> include
>>>>>> that
>>>>>>>> in
>>>>>>>>>> the
>>>>>>>>>>>>>>>>> macro,
>>>>>>>>>>>>>>>>>>>>> too.)
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> But overall, I consider this a detail and am as
>> fine
>>>>>> with
>>>>>>>> your
>>>>>>>>>>>>>>>>>>>>>>>> solution
>>>>>>>>>>>>>>>>>>>>>>>> as with the SUB() macros. What is better is that
>>>> the
>>>>>>>> linux=1
>>>>>>>>>>>> etc
>>>>>>>>>>>>>>>>>>>>>>>> problems are avoided.
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> Any more opinions whether the macros should
>> be
>>>>>>>> upper
>>>>>>>>>> case?
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> Yeah they probably should be.
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>>>> David
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> Best regards,
>>>>>>>>>>>>>>>>>>>>>>>> Goetz.
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>>>>>>>>>>>>>>> From: Kim Barrett
>>>> [mailto:kim.barrett at oracle.com]
>>>>>>>>>>>>>>>>>>>>>>>>> Sent: Donnerstag, 14. Juli 2016 23:20
>>>>>>>>>>>>>>>>>>>>>>>>> To: Lindenmaier, Goetz
>>>>>>>> <goetz.lindenmaier at sap.com>
>>>>>>>>>>>>>>>>>>>>>>>>> Cc: Andrew Haley <aph at redhat.com>;
>> hotspot-
>>>>>>>> compiler-
>>>>>>>>>>>>>>>>>>>>>>>>> dev at openjdk.java.net; hotspot-runtime-
>>>>>>>>>>>>>> dev at openjdk.java.net
>>>>>>>>>>>>>>>>>>>>>>>>> Subject: Re: RFR(L): 8161259: Simplify including
>>>>>>>> platform
>>>>>>>>>>>> files.
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> On Jul 14, 2016, at 3:18 PM, Lindenmaier,
>> Goetz
>>>>>>>>>>>>>>>>>>>>>>>>> <goetz.lindenmaier at sap.com> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> Hi everybody,
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> please take into account that these macros
>> are
>>>>>> only
>>>>>>>>>> used
>>>>>>>>>>>>>> within
>>>>>>>>>>>>>>>>> 20
>>>>>>>>>>>>>>>>>>>>> lines
>>>>>>>>>>>>>>>>>>>>>>> of
>>>>>>>>>>>>>>>>>>>>>>>>>> the file macro.hpp. The code everybody
>> needs
>>>> to
>>>>>>>>>>>> understand
>>>>>>>>>>>>>> is
>>>>>>>>>>>>>>>>>>>>>>>>>> #include cpu_header(bytes)
>>>>>>>>>>>>>>>>>>>>>>>>>> which, in this example, is in file bytes.hpp
>> and
>>>>>>>> expands to
>>>>>>>>>>>>>>>>>>>>>>>>> bytes_<cpu>.hpp.
>>>>>>>>>>>>>>>>>>>>>>>>>> There are six of these, for cpu/os/os_cpu
>>>>>>>>>>>> and .hpp/.inline.hpp.
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> I really would appreciate if I don't have to
>>>> spend
>>>>>> days
>>>>>>>>>>>>>>>>>>>>>>>>>> editing the
>>>>>>>>>>>>>>>>>>>>>>>>>> 12 lines that use SUB().
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> @Kim
>>>>>>>>>>>>>>>>>>>>>>>>>> I'm working on the s390 port, and posted my
>>>>>> current
>>>>>>>>>>>> progress
>>>>>>>>>>>>>>>>>>> claiming
>>>>>>>>>>>>>>>>>>>>>>>>>> that the biggest shared change I need to do
>> is
>>>>>> adding
>>>>>>>> the
>>>>>>>>>>>>>>>>> #includes.
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>> http://mail.openjdk.java.net/pipermail/hotspot-
>>>>>>>>>> dev/2016-
>>>>>>>>>>>>>>>>>>>>>>>>> July/023782.html
>>>>>>>>>>>>>>>>>>>>>>>>>> This arose the discussion about the includes.
>>>>>>>>>>>>>>>>>>>>>>>>>> Later I posted a prototype of what Volker
>>>>>> proposed
>>>>>>>> in
>>>>>>>>>> that
>>>>>>>>>>>>>>>>>>> discussion
>>>>>>>>>>>>>>>>>>>>>>>>>> to that thread:
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>> http://mail.openjdk.java.net/pipermail/hotspot-
>>>>>>>>>> dev/2016-
>>>>>>>>>>>>>>>>>>>>>>>>> July/023934.html
>>>>>>>>>>>>>>>>>>>>>>>>>> which I now turned into a RFR.
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> Thanks.
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> I think I see what happened to the email
>> thread
>>>> for
>>>>>>>> me; it
>>>>>>>>>>>> looks
>>>>>>>>>>>>>>>>> like
>>>>>>>>>>>>>>>>>>> one
>>>>>>>>>>>>>>>>>>>>> of
>>>>>>>>>>>>>>>>>>>>>>>>> Andrew’s replies went to hotspot-compiler-
>> dev
>>>> but
>>>>>>>> not
>>>>>>>>>>>>>> hotspot-
>>>>>>>>>>>>>>>>>>>>> runtime-
>>>>>>>>>>>>>>>>>>>>>>>>> dev,
>>>>>>>>>>>>>>>>>>>>>>>>> and I was not subscribed to the compiler list
>> this
>>>>>>>> morning.
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> I like the idea, just quibbling over details. I've
>>>> only
>>>>>>>>>>>>>>>>>>>>>>>>> reviewed
>>>>>>>>>>>>>>>>>>>>>>>>> macros.hpp so far; the rest looks like it can
>> wait
>>>>>> until
>>>>>>>> the
>>>>>>>>>>>>>>>>>>>>>>>>> details
>>>>>>>>>>>>>>>>>>>>>>>>> are settled.
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> --------------------------------------------------------
>> ---
>>>> ----
>>>>>> ---
>>>>>>>> ----
>>>>>>>>>> ---
>>>>>>>>>>>> ---
>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> src/share/vm/utilities/macros.hpp
>>>>>>>>>>>>>>>>>>>>>>>>> 470 // Helper macros to workaround existing
>>>>>> #defines
>>>>>>>>>> that
>>>>>>>>>>>>>> spoil
>>>>>>>>>>>>>>>>>>>>>>>>> 471 // the macro expansion. Detected so far:
>>>>>> linux=1,
>>>>>>>>>>>> sparc=1.
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> This issue can be dodged by making the
>> leading
>>>>>>>>>> underscore
>>>>>>>>>>>>>> part
>>>>>>>>>>>>>>>>> of
>>>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>>>>>>> expansion for INCLUDE_SUFFIX_*, e.g. in
>>>>>>>>>>>>>>>>>>> make/lib/CompileJvm.gmk,
>>>>>>>>>>>>>>>>>>>>> use
>>>>>>>>>>>>>>>>>>>>>>>>> instead
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> 66 JVM_CFLAGS_TARGET_DEFINES += \
>>>>>>>>>>>>>>>>>>>>>>>>> 67 -
>>>>>>>> DINCLUDE_SUFFIX_OS=_$(HOTSPOT_TARGET_OS)
>>>>>>>>>> \
>>>>>>>>>>>>>>>>>>>>>>>>> 68 -
>>>>>>>>>>>>>>>>>
>>>> DINCLUDE_SUFFIX_CPU=_$(HOTSPOT_TARGET_CPU_ARCH)
>>>>>>>>>>>>>>>>>>> \
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> Note the insertion of leading "_" for the
>> values.
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> Then the whole macro block can be written as
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> #define cpu_header_stem(basename)
>>>>>>>>>>>>>>>>> PASTE_TOKENS(basename,
>>>>>>>>>>>>>>>>>>>>>>>>> INCLUDE_SUFFIX_CPU)
>>>>>>>>>>>>>>>>>>>>>>>>> #define os_header_stem(basename)
>>>>>>>>>>>>>> PASTE_TOKENS(basename,
>>>>>>>>>>>>>>>>>>>>>>>>> INCLUDE_SUFFIX_OS)
>>>>>>>>>>>>>>>>>>>>>>>>> #define os_cpu_header_stem(basename)
>>>>>>>>>>>>>>>>>>> PASTE_TOKENS(basename,
>>>>>>>>>>>>>>>>>>>>>>>>> PASTE_TOKENS(INCLUDE_SUFFIX_OS,
>>>>>>>>>>>> INCLUDE_SUFFIX_CPU))
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> #define cpu_header(basename)
>>>>>>>>>>>>>>>>>>>>>>> XSTR(cpu_header_stem(basename).hpp)
>>>>>>>>>>>>>>>>>>>>>>>>> #define cpu_header_inline(basename)
>>>>>>>>>>>>>>>>>>>>>>>>>
>> XSTR(cpu_header_stem(basename).inline.hpp)
>>>>>>>>>>>>>>>>>>>>>>>>> #define os_header(basename)
>>>>>>>>>>>>>>>>>>>>> XSTR(os_header_stem(basename).hpp)
>>>>>>>>>>>>>>>>>>>>>>>>> #define os_header_inline(basename)
>>>>>>>>>>>>>>>>>>>>>>>>>
>> XSTR(os_header_stem(basename).inline.hpp)
>>>>>>>>>>>>>>>>>>>>>>>>> #define os_cpu_header(basename)
>>>>>>>>>>>>>>>>>>>>>>>>> XSTR(os_cpu_header_stem(basename).hpp)
>>>>>>>>>>>>>>>>>>>>>>>>> #define os_cpu_header_inline(basename)
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>> XSTR(os_cpu_header_stem(basename).inline.hpp)
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> And SUB is no longer used...
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> If some future build system wants brackets
>>>> instead
>>>>>> of
>>>>>>>>>>>>>>>>>>>>>>>>> strings, just
>>>>>>>>>>>>>>>>>>>>>>>>> replace XSTR(...) with <...>.
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> We lose if _linux or _sparc (for example) are
>>>>>> defined,
>>>>>>>> but
>>>>>>>>>>>>>>>>>>>>>>>>> that's
>>>>>>>>>>>>>>>>> true
>>>>>>>>>>>>>>>>>>>>>>>>> for the webrev.01 code too. But note that
>>>>>> underscore
>>>>>>>>>>>>>> followed
>>>>>>>>>>>>>>>>> by a
>>>>>>>>>>>>>>>>>>>>>>>>> lowercase letter is not in the reserved word
>>>> pattern
>>>>>>>> for
>>>>>>>>>>>> C/C++.
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> BTW, my preference would be to use
>> uppercase
>>>> for
>>>>>>>> the
>>>>>>>>>>>> macro
>>>>>>>>>>>>>>>>>>> names.
>>>>>>>>>>>>>>>>>>>>> I
>>>>>>>>>>>>>>>>>>>>>>>>> don't know what others think about that.
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> --------------------------------------------------------
>> ---
>>>> ----
>>>>>> ---
>>>>>>>> ----
>>>>>>>>>> ---
>>>>>>>>>>>> ---
>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
More information about the hotspot-runtime-dev
mailing list