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

[Coleen Phillimore]

http://cr.openjdk.java.net/~goetz/wr16/8161259-newPfmIncl/webrev.02/src/cpu/aarch64/vm/vm_version_aarch64.cpp.udiff.html

-#ifdef TARGET_OS_FAMILY_linux

-# include "os_linux.inline.hpp"
-#endif
+
+#include OS_HEADER(os)


Why isn't this OS_HEADER_INLINE (if there's such a macro), since it's 
including the .inline.hpp file?

Why is there just this single #include os_linux.inline.hpp and not the 
cascade of os_aix.hpp files?

For the record, the original intent on having these cascading #include 
directives and not having dispatch .hpp files (ala creating an 
"os_cpu.hpp" file for example) was so that eventually all the #include 
directives would be pushed down to the lowest level that needed it, and 
everything would be super-local and clean :)   For example, the only cpp 
files that needed anything in assembler_x86.hpp would be already located 
in the src/cpu/x86/vm directory and could include assembler_x86.hpp 
directly.   I still think this is a worthwhile goal but from reading 
this, the sources are far from this.

I also didn't want us to have to create empty platform files (but I see 
some were added here).   Maybe macroAssembler_cpu.inline.hpp inclusion 
can be pushed down after this change as a further cleanup.

This looks great though, and sorely needed, especially when adding a new 
platform or cpu implementation.

Thanks,
Coleen

On 7/15/16 8:17 AM, 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/
>
> 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.
>>>>
>>>> ------------------------------------------------------------------------------