RFR(L): 8161259: Simplify including platform files.
Daniel D. Daugherty
daniel.daugherty at oracle.com
Tue Jul 26 21:38:03 UTC 2016
I've made some manual updates to both bugs and will keep an eye
on JDK-8161258. As it goes through it's phases I'll try to make
the same updates to JDK-8161259.
Dan
On 7/26/16 2:21 PM, David Holmes wrote:
> The patch for this used the wrong bug ID - 8161258 instead of 8161259.
>
> Not sure what I'm going to be able to do about that :(
>
> David
> -----
>
> On 26/07/2016 11:12 AM, David Holmes wrote:
>> 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