s390x port progress: patch queue for hotspot assembled.
Lindenmaier, Goetz
goetz.lindenmaier at sap.com
Wed Jul 13 07:00:52 UTC 2016
Hi David,
> Is there a reason you didn't do what I had suggested with the above and
> change the last _ to = ?
* I think the names should match what is in the path, therefore I think
CPU is better than ARCH (we have src/cpu/x86).
* I left out "FAMILY". I don't see any gain in this part of the name.
* Three of them are superfluous anyways.
I thought about calling them INCLUDE_OS and INCLUDE_CPU, that
better indicates what they are meant for.
I also thought about only defining them in the macros.hpp file. We
have -DLINUX -DAMD64 / -DAIX -DPPC64 etc. on the command line
to identify which port to compile.
> I thought my suggestion would avoid the issue
> with the LINUX=1 problem. ?
Do you want me to do -DTARGET_OS_FAMILY=_linux ? with '_'?
The problem is that there is lower case #define linux=1.
I don't think we should define things with leading '_', they are often
used by the system headers / compilers.
I rebased my changes, that did not cause any conflicts. I also downloaded
the changesets and patched them into my repo, that works too.
You first need to apply the cleanups, then the new includes.
Best regards,
Goetz.
> -----Original Message-----
> From: David Holmes [mailto:david.holmes at oracle.com]
> Sent: Mittwoch, 13. Juli 2016 05:43
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>;
> 'dean.long at oracle.com' <dean.long at oracle.com>; hotspot-
> dev at openjdk.java.net
> Subject: Re: s390x port progress: patch queue for hotspot assembled.
>
> 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