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

[Lindenmaier, Goetz]

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/

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.
> >>
> >> ------------------------------------------------------------------------------
> >