RFR(L): 8161259: Simplify including platform files.
David Holmes
david.holmes at oracle.com
Tue Jul 26 20:21:10 UTC 2016
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