s390x port progress: patch queue for hotspot assembled.

David Holmes david.holmes at oracle.com
Wed Jul 13 03:43:02 UTC 2016 

Hi Goetz,

Thanks for doing this! I think it looks good. Only nit ...

On 13/07/2016 7:58 AM, Lindenmaier, Goetz wrote:
> Hi,
>
> I implemented a prototype of the include model proposed by Volker.
> I tested this so far on linux_x86_64, aix_ppc64 and linux_ppc64.
> I'll test it on the other OSes/CPUs I have available tomorrow.
>
> The direcitves
>   -DTARGET_OS_FAMILY_linux
>   -DTARGET_ARCH_MODEL_x86_64
>   -DTARGET_ARCH_x86
>   -DTARGET_OS_ARCH_MODEL_linux_x86_64
>   -DTARGET_OS_ARCH_linux_x86
> are replaced by -DTARGET_OS=linux -DTARGET_CPU=x86.
> (OS and CPU are used for vm_version.cpp)

Is there a reason you didn't do what I had suggested with the above and 
change the last _ to = ? I thought my suggestion would avoid the issue 
with the LINUX=1 problem. ??

I was going to test this out with our other platforms but unfortunately 
the patch didn't apply on latest jdk9/hs.

Thanks,
David

> The macros proposed by Volker are in utilities/macros.hpp.
>
> There are some places where above TARGET_... macros are used for other
> purposes than guarding the includes, I replaced them to use the other
> platform dependent macros already defined.
>
> I also removed all headers carying _64 or _32 in their name.
> There were three on ppc. As I learned there is no PPC32 port
> any more, so changing this should be fine. I found one on x86 (stubRoutines...hpp).
> I merged it and use LP64 guards for the differences. I think this is
> acceptable as the files are rather small and it's the only case.
>
> The adlc generated files all had suffix _64/_32 which I removed.
> These are not significant, either.
>
> All this normalizing edits can be found in this webrev. It would be
> useful to apply them anyways:
> http://cr.openjdk.java.net/~goetz/wr16/newPfmIncl/webrev-cleanups/
> The real change is here:
> http://cr.openjdk.java.net/~goetz/wr16/newPfmIncl/webrev-newIncludes/
> I had to do a tweak for the #define linux=1 in macros.hpp.
>
> Please have a look at these changes. Also, check whether this
> works fine with the IDE of your choice :)
>
> If this finds appreciation, I'll open an Enhancement and make a
> real webrev/RFR.
>
> Best regards,
>   Goetz.
>
>
>
>
>
>
>
>> -----Original Message-----
>> From: hotspot-dev [mailto:hotspot-dev-bounces at openjdk.java.net] On
>> Behalf Of dean.long at oracle.com
>> Sent: Wednesday, July 06, 2016 7:49 AM
>> To: hotspot-dev at openjdk.java.net
>> Subject: Re: s390x port progress: patch queue for hotspot assembled.
>>
>> On 7/5/16 10:42 PM, dean.long at oracle.com wrote:
>>
>>> On 7/4/16 2:59 PM, David Holmes wrote:
>>>
>>>> On 5/07/2016 7:44 AM, Stefan Karlsson wrote:
>>>>> On 2016-07-04 23:03, David Holmes wrote:
>>>>>> On 5/07/2016 12:07 AM, Volker Simonis wrote:
>>>>>>> On Mon, Jul 4, 2016 at 3:17 PM, Andrew Haley <aph at redhat.com>
>> wrote:
>>>>>>>> On 04/07/16 12:22, David Holmes wrote:
>>>>>>>>
>>>>>>>>> Unless the files are included into more than one other file then
>>>>>>>>> adding
>>>>>>>>> a level of indirection doesn't really help anything.
>>>>>>>>
>>>>>>>> I'm not sure what this means.
>>>>>>>>
>>>>>>>>> But I don't see why the platform part of the name needs to be kept
>>>>>>>>> when
>>>>>>>>> it is already part of the path?
>>>>>>>>
>>>>>>>> When switching between files in an editor, one doesn't usually
>>>>>>>> type in the full path, but the filename.  I'd be very sad to lose
>>>>>>>> that, and I think that unique filenames are a nice thing to have.
>>>>>>>>
>>>>>>>
>>>>>>> In Emacs you can try the uniquify [1] extension which would solve the
>>>>>>> problem. I already use it for editing various hotspot version (i.e.
>>>>>>> hs-comp, jdk9-dev) in one Emacs. But I agree that Emacs is probably
>>>>>>> not a practical solution for everybody :)
>>>>>>>
>>>>>>> [1]
>>>>>>>
>> https://www.gnu.org/software/emacs/manual/html_node/emacs/Uniquify.
>> html
>>>>>>>
>>>>>>>
>>>>>>>> Andrew.
>>>>>>>>
>>>>>>>
>>>>>>> We could try something like this (adapted from [2]) although I'm not
>>>>>>> sure if that's really a good solution (i.e. if it is treated right by
>>>>>>> various IDEs):
>>>>>>>
>>>>>>> include_test.cpp
>>>>>>> ============
>>>>>>>
>>>>>>> #define xstr(x) #x
>>>>>>> #define str(x) xstr(x)
>>>>>>> #define sub(x) x
>>>>>>> #define cpu_header(x) str(sub(x)sub(_)CPU.hpp)
>>>>>>> #define os_header(x) str(sub(x)sub(_)OS.hpp)
>>>>>>> #define os_cpu_header(x) str(sub(x)sub(_)sub(OS)sub(_)CPU.hpp)
>>>>>>>
>>>>>>> #include cpu_header(assembler)
>>>>>>> #include os_header(assembler)
>>>>>>> #include os_cpu_header(assembler)
>>>>>>
>>>>>> This is the parameterization I thought was not possible. For some
>>>>>> reason I thought #define'd values could not be used within #include
>>>>>> directives.
>>>>>
>>>>> FWIW, we tried a similar approach when includeDB was removed, but
>>>>> abandoned it because of the problem described below with the 'linux'
>>>>> define.
>>>>
>>>> Give we have in the build (eg for linux x86) presently:
>>>>
>>>> -DTARGET_OS_FAMILY_linux
>>>> -DTARGET_ARCH_MODEL_x86_32
>>>> -DTARGET_ARCH_x86
>>>> -DTARGET_OS_ARCH_MODEL_linux_x86_32
>>>> -DTARGET_OS_ARCH_linux_x86
>>>>
>>>> I wonder if we can simply replace the above with:
>>>>
>>>> -DTARGET_OS_FAMILY=linux
>>>> -DTARGET_ARCH_MODEL=x86_32
>>>> -DTARGET_ARCH=x86
>>>> -DTARGET_OS_ARCH_MODEL=linux_x86_32
>>>> -DTARGET_OS_ARCH=linux_x86
>>>>
>>>> and use those variables for the include directives?
>>>>
>>>
>>> I remember experimenting with that idea, but gave up because I
>>> couldn't get it work.
>>> What I ended up doing was something like this:
>>>
>>> #*include* C1_LIRGENERATOR_MD_HPP
>>>
>>
>> Oops, let's try that again:
>>
>> #include C1_LIRGENERATOR_MD_HPP
>>
>> where, using x86 as an example, macros like C1_LIRGENERATOR_MD_HPP
>> are
>> defined in globalDefinitions_x86.hpp:
>>
>> #define C1_LIRGENERATOR_MD_HPP "c1_LIRGenerator_x86.hpp"
>>
>> dl
>>
>>> I don't know if this confuses IDEs or not.
>>>
>>> dl
>>>
>>>> David
>>>>
>>>>> StefanK
>>>>>
>>>>>>
>>>>>> This looks very workable to me.
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>>
>>>>>>> assembler_s390x.hpp
>>>>>>> ================
>>>>>>> #pragma message ("including \"assembler_s390x.h\"")
>>>>>>>
>>>>>>> assembler_linux.hpp
>>>>>>> ===============
>>>>>>> #pragma message ("including \"assembler_linux.h\"")
>>>>>>>
>>>>>>> assembler_linux_s390x.hpp
>>>>>>> ====================
>>>>>>> #pragma message ("including \"assembler_linux_s390x.h\"")
>>>>>>>
>>>>>>>
>>>>>>> It works on all the platforms I have tested so far (see below) except
>>>>>>> Linux where it needs some tweaks (see comments below):
>>>>>>>
>>>>>>>
>>>>>>> Solaris:
>>>>>>> ======
>>>>>>> /SS12u4/SUNWspro/bin/CC -DCPU=s390x -DOS="linux" -E
>> include_test.cpp
>>>>>>> #1 "include_test.cpp"
>>>>>>> #1 "assembler_s390x.hpp"
>>>>>>> #pragma message ( "including \"assembler_s390x.h\"" )
>>>>>>> #1 "assembler_linux.hpp"
>>>>>>> #pragma message ( "including \"assembler_linux.h\"" )
>>>>>>> #1 "assembler_linux_s390x.hpp"
>>>>>>> #pragma message ( "including \"assembler_linux_s390x.h\"" )
>>>>>>>
>>>>>>> AIX:
>>>>>>> ===
>>>>>>> /usr/vacpp_V12/usr/vacpp/bin/xlC_r -DCPU=s390x -DOS=linux -c
>>>>>>> include_test.cpp
>>>>>>> "assembler_s390x.hpp", line 1.9: 1540-1401 (I) An unknown "pragma
>>>>>>> message" is specified.
>>>>>>> "assembler_linux.hpp", line 1.9: 1540-1401 (I) An unknown "pragma
>>>>>>> message" is specified.
>>>>>>> "assembler_linux_s390x.hpp", line 1.9: 1540-1401 (I) An unknown
>>>>>>> "pragma message" is specified.
>>>>>>>
>>>>>>> MacOS X
>>>>>>> =======
>>>>>>> /usr/bin/clang++ -DCPU=s390x -DOS=linux -c include_test.cpp
>>>>>>> In file included from include_test.cpp:8:
>>>>>>> ./assembler_s390x.hpp:1:9: warning: including "assembler_s390x.h"
>>>>>>> [-W#pragma-messages]
>>>>>>> #pragma message ("including \"assembler_s390x.h\"")
>>>>>>>         ^
>>>>>>> In file included from include_test.cpp:9:
>>>>>>> ./assembler_linux.hpp:1:9: warning: including "assembler_linux.h"
>>>>>>> [-W#pragma-messages]
>>>>>>> #pragma message ("including \"assembler_linux.h\"")
>>>>>>>         ^
>>>>>>> In file included from include_test.cpp:10:
>>>>>>> ./assembler_linux_s390x.hpp:1:9: warning: including
>>>>>>> "assembler_linux_s390x.h" [-W#pragma-messages]
>>>>>>> #pragma message ("including \"assembler_linux_s390x.h\"")
>>>>>>>         ^
>>>>>>> 3 warnings generated.
>>>>>>>
>>>>>>> Windows:
>>>>>>> =======
>>>>>>> $ /cygdrive/c/progra~2/micros~1.0/vc/bin/amd64/cl -DCPU=s390x
>>>>>>> -DOS=linux -c include_test.
>>>>>>> cpp
>>>>>>> Microsoft (R) C/C++ Optimizing Compiler Version 16.00.40219.01 for
>>>>>>> x64
>>>>>>> Copyright (C) Microsoft Corporation.  All rights reserved.
>>>>>>>
>>>>>>> include_test.cpp
>>>>>>> including "assembler_s390x.h"
>>>>>>> including "assembler_linux.h"
>>>>>>> including "assembler_linux_s390x.h"
>>>>>>>
>>>>>>> HPUX:
>>>>>>> =====
>>>>>>> $ /opt/aCC/bin/aCC -DCPU=s390x -DOS=linux -c include_test.cpp
>>>>>>> Info #4087-D: "including \"assembler_s390x.h\""
>>>>>>>
>>>>>>> Info #4087-D: "including \"assembler_linux.h\""
>>>>>>>
>>>>>>> Info #4087-D: "including \"assembler_linux_s390x.h\""
>>>>>>>
>>>>>>> Linux:
>>>>>>> =====
>>>>>>> g++ -DCPU=s390x -DOS=linux -c include_test.cpp
>>>>>>> include_test.cpp:9:30: fatal error: assembler_1.hpp: No such file or
>>>>>>> directory
>>>>>>> compilation terminated.
>>>>>>>
>>>>>>> The problem on Linux is that 'linux' is already predefined to '1' but
>>>>>>> that could be easily fixed for example by slightly changing the name
>>>>>>> or by adding the underscore to the definiton of the OS macro (i.e.
>>>>>>> -DOS=_linux).
>>>>>>>
>>>>>>> As I said, I'm still not sure if this looks nice? I just wanted to
>>>>>>> post my findings for discussion :)
>>>>>>>
>>>>>>> Regards,
>>>>>>> Volker
>>>>>>>
>>>>>>> [2]
>>>>>>> http://stackoverflow.com/questions/1852652/how-can-i-include-a-
>> file-whose-name-is-built-up-from-a-macro
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>
>>>
>

More information about the hotspot-dev mailing list