RFR(L): 8161259: Simplify including platform files.
Lindenmaier, Goetz
goetz.lindenmaier at sap.com
Tue Jul 26 07:23:58 UTC 2016
Great, thanks a lot again!
Best regards,
Goetz.
> -----Original Message-----
> From: David Holmes [mailto:david.holmes at oracle.com]
> Sent: Dienstag, 26. Juli 2016 03:13
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.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 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