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

[Volker Simonis]

On Tue, Oct 25, 2016 at 5:58 AM, David Holmes <david.holmes at oracle.com> wrote:
> On 25/10/2016 1:36 AM, Volker Simonis 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.
>
>
> Seems okay.
>
>> I also had to remove some AIX-specific constant definitions and
>> replace them by the more standard '<number>*[K,M,G]' notation.
>
>
> Assuming AIX is 64-bit only this seems okay - the numeric values have
> changed from uint64_t to size_t, which should still be a 64-bit unsigned
> value.

You're right but I hope we'll never have to do a 32-bit AIX port :)
>
> BTW I'm traveling tomorrow so would not be able to sponsor this till late
> Wednesday at the earliest.
>

That would be still acceptable :) and very much appreciated!

Thanks,
Volker

> Thanks,
> David
>
>
>> 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
>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>