RFR(L): 8161259: Simplify including platform files.
David Holmes
david.holmes at oracle.com
Tue Jul 26 01:12:55 UTC 2016
On 25/07/2016 3:01 PM, David Holmes wrote:
> This all looks good. Approval is now in place. Just waiting for final
> conformation this builds okay for Aarch64 folk.
That confirmation was sent to compiler-dev so the changes have been pushed.
David
> Thanks,
> David
>
> On 21/07/2016 8:59 PM, Lindenmaier, Goetz wrote:
>> Hi,
>>
>>
>>> I see the typo was actually much bigger than I thought :) Presumably the
>> Well, I edited it some more ... this time I replaced the webrev in-place,
>> webrev.05 is fixed now.
>>
>> I really should have an aarch64 machine :)
>>
>> Best regards,
>> Goetz.
>>
>>> -----Original Message-----
>>> From: David Holmes [mailto:david.holmes at oracle.com]
>>> Sent: Donnerstag, 21. Juli 2016 12:00
>>> 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 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-compiler-dev
mailing list