RFR(L): 8161259: Simplify including platform files.
Lindenmaier, Goetz
goetz.lindenmaier at sap.com
Wed Jul 20 09:21:41 UTC 2016
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.
For the records, openJdk aarch64 has a C1 port. And yes, such cleanups should
not be in this change.
Thanks for doing the closed changes!
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