RFR(XS): 8168490: Use the LL/ULL suffixes to define 64-bit integer literals on Windows

[Thomas Stüfe]

Hi Volker,

so you didn't like my old SIZE_.. macros huh :)

The changes are fine, and strictly speaking make the AIX port more correct,
because before it was assumed all over the place that size_t is 64bit, and
now we use the correct size_t typed constants. So should we ever do an AIX
32bit port this will help :)

Nitpicks:

- globals_aix.hpp: you can remove the UCONST64 macro on
MaxExpectedDataSegmentSize.
- virtualspace.cpp: can you remove pls also remove the SIZE_xx definitions
and usage around get_attach_addresses_for_disjoint_mode()?

Thanks, Thomas


On Mon, Oct 24, 2016 at 5:36 PM, Volker Simonis <volker.simonis at gmail.com>
wrote:

> So here comes the new webrev:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2016/8168490.v2/
>
> It moves the definition of CONST64/UCONST64 and min_jlong/max_jlong
> from the compiler-specific files to globalDefinitions.hpp.
>
> I also had to remove some AIX-specific constant definitions and
> replace them by the more standard '<number>*[K,M,G]' notation.
>
> I did build and smoke tested this on Linux/amd64, MaxOS X, AIX,
> Solaris and Windows.
>
> Thanks,
> Volker
>
>
> On Mon, Oct 24, 2016 at 2:51 PM, Volker Simonis
> <volker.simonis at gmail.com> wrote:
> > Hi Mikael,
> >
> > you're right - the OpenJDK now has the same definitions for
> > CONST64/UCONST64 on all platforms. If you don't have other settings
> > for you're closed ports I can certainly generalize the definition into
> > globalDefinitions.hpp (it also works for HP-UX which is our last
> > closed platform :)
> >
> > Notice that this would also require to move the definition of
> > min_jlong/max_jlong from the compiler-specific files to
> > globalDefinitions.hpp.
> >
> > I can do a new webrev with these additional changes.
> >
> > Regards,
> > Volker
> >
> >
> > On Mon, Oct 24, 2016 at 11:21 AM, Mikael Gerdin
> > <mikael.gerdin at oracle.com> wrote:
> >> Hi Volker,
> >>
> >>
> >> On 2016-10-24 10:53, Volker Simonis wrote:
> >>>
> >>> On Mon, Oct 24, 2016 at 9:14 AM, David Holmes <david.holmes at oracle.com
> >
> >>> wrote:
> >>>>
> >>>> On 24/10/2016 5:12 PM, Volker Simonis wrote:
> >>>>>
> >>>>>
> >>>>> On Mon, Oct 24, 2016 at 2:43 AM, David Holmes <
> david.holmes at oracle.com>
> >>>>> wrote:
> >>>>>>
> >>>>>>
> >>>>>> Hi Volker,
> >>>>>>
> >>>>>> On 22/10/2016 1:35 AM, Volker Simonis wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> Hi,
> >>>>>>>
> >>>>>>> can I please have a review and sponsor for the following trivial
> fix:
> >>>>>>>
> >>>>>>> http://cr.openjdk.java.net/~simonis/webrevs/2016/8168490/
> >>>>>>> https://bugs.openjdk.java.net/browse/JDK-8168490
> >>>>>>>
> >>>>>>> Currently we use the i64/ui64 suffixes to declare 64-bit integer
> >>>>>>> literals on Windows (see
> >>>>>>> src/share/vm/utilities/globalDefinitions_visCPP.hpp).
> >>>>>>>
> >>>>>>> Unfortunately this suffix is not known to Eclipse so most of a
> hotspot
> >>>>>>> files have errors in an Eclipse project (e.g. NULL is defined as
> >>>>>>> '0i64' which gives a syntax error in Eclipse, but NULL is used in
> most
> >>>>>>> hotspot of the source files).
> >>>>>>>
> >>>>>>> Fortunately VS supports the more standard conforming LL/ULL
> suffixes
> >>>>>>> (see http://en.cppreference.com/w/cpp/language/integer_literal)
> since
> >>>>>>> at least VS2010 (see
> >>>>>>> https://msdn.microsoft.com/en-us/library/00a1awxf(v=vs.100).
> aspx).I
> >>>>>>> therefore propose to change the suffix for integer literals from
> >>>>>>> i64/ui64 to LL/ULL on Windows.
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> In the sense that this changes the VIS specific code to use a more
> >>>>>> stand
> >>>>>> conforming form this seems fine.
> >>>>>>
> >>>>>> But as I wrote in the bug report, should we even be using this file
> >>>>>> with
> >>>>>> non
> >>>>>> Visual Studio compilers?
> >>>>>>
> >>>>>
> >>>>> What I meant with "Eclipse doesn't understand" the suffix is that the
> >>>>> Eclipse C++ parser doesn't understand it. Eclipse just parses all the
> >>>>> C++ files which belong to a project in order to build up its code
> >>>>> database for code navigation. As such it reads
> >>>>> globalDefinitions_visCPP.hpp on Windows (if we configure a
> >>>>> corresponding project) or  globalDefinitions_gcc.hpp on Linux, etc...
> >>>>>
> >>>>> This does not mean that we are using globalDefinitions_visCPP.hpp for
> >>>>> building with another compiler. And we have no IDE-specific headers
> >>>>> (this actually makes no sense since the IDEs should use the same
> >>>>> settings which are used for the actual builds).
> >>>>>
> >>>>> Not sure about other IDEs, but Eclipse's C++ parser doesn't
> understand
> >>>>> the i64/ui64 suffix and as there is a better, more
> standard-conforming
> >>>>> way of declaring 64-bit integer literals on Windows I've just
> proposed
> >>>>> the cleanup.
> >>>>>
> >>>>> Does this answer your question?
> >>>>
> >>>>
> >>>>
> >>>> Yes - thanks for clarifying.
> >>>>
> >>>
> >>> Your welcome! Could you then please sponsor this change :)
> >>
> >>
> >> Does this change make the definition of CONST64 / UCONST64 the same on
> all
> >> globalDefinitions_x variants?
> >> If so can we move the definition to the common file instead of having
> it in
> >> the per-compiler files?
> >>
> >> Thanks
> >> /Mikael
> >>
> >>
> >>>
> >>>>
> >>>> David
> >>>>
> >>>>> Thank you and best egards,
> >>>>> Volker
> >>>>>
> >>>>>
> >>>>>> Thanks,
> >>>>>> David
> >>>>>>
> >>>>>>
> >>>>>>> Thank you and best regards,
> >>>>>>> Volker
> >>>>>>>
> >>>>>>
> >>>>
> >>
>