RFR(L): 8161259: Simplify including platform files.
Lindenmaier, Goetz
goetz.lindenmaier at sap.com
Thu Jul 21 09:27:45 UTC 2016
Hi David,
Copyright of register_definitions_x86.cpp is already fixed in hs-comp,
I'll skip it to avoid merges.
I fixed the others. New webrevs, also with Coleens fixes:
http://cr.openjdk.java.net/~goetz/wr16/8161259-newPfmIncl/webrev.05/
I also did another zero build configured as follows:
--disable-hotspot-gtest --with-debug-level=fastdebug --with-jvm-variants=zero
on linuxx86_64.
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