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