RFR(L): 8161259: Simplify including platform files.

David Holmes david.holmes at oracle.com
Tue Jul 19 11:23:23 UTC 2016 

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