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

[David Holmes]

Looks good.

Submitting the push job now.

Thanks,
David

On 26/10/2016 12:57 AM, Volker Simonis wrote:
> On Tue, Oct 25, 2016 at 8:09 AM, Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
>> Hi Volker,
>>
>> so you didn't like my old SIZE_.. macros huh :)
>>
>
> I loved them :) but it was not so easy to keep them in the compiler
> specifix _xlc file now that the CONST64/UCONST64 macros are defined in
> the general globalDefinitions.hpp file.
>
>> 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.
>
> I fixed that. Without UCONST64 it is only correct on 64-bit platforms
> but it looks nicer and I doubt we'll ever do a 32bit AIX port :)
>
>> - virtualspace.cpp: can you remove pls also remove the SIZE_xx definitions
>> and usage around get_attach_addresses_for_disjoint_mode()?
>>
>
> I'd rather leave that for another clean-up because it is not really
> related to this change.
>
> Following the hopefully final webrev:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2016/8168490.v3/
>
> Thanks,
> Volker
>
>> 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
>>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>
>>
>>