Request for review: 7102489: RFE: cleanup jlong typedef on __APPLE__ and _LP64 systems

Coleen Phillimore coleen.phillimore at
Wed Jan 16 18:46:01 PST 2013


I really think Harold's change is good.  It takes the APPLE conditional 
compilation out of the definition of jlong and moves APPLE specific to 
the definition of JLONG_FORMAT.    The jlong typedef for macosx now 
matches linux and solaris on x86, and it matches the typedef that's in 
the JDK in jni_md.h.   Changing this vs. cleaning up the conditional 
code and printing is a much bigger undertaking and might need a CCC 
request.   This shouldn't prevent the cleanup.


On 1/16/2013 4:51 PM, harold seigel wrote:
> Hi David,
> Thanks for your comments.  Please see my interspersed comments.
> Also, I posted a modified webrev at 
> <> that has 32 and 
> 64 definitions of JLONG_FORMAT in .../launcher/java_md.h and 2013 
> copyrights.
> Thanks, Harold
> On 1/16/2013 1:53 AM, David Holmes wrote:
>> On 16/01/2013 9:12 AM, Coleen Phillimore wrote:
>>> I would really be worried about changing the definition in hotspot of
>>> jlong.  That seems very risky.
>> No risk. jlong is a 64-bit signed value regardless. And int64_t will 
>> be either long or "long long" as appropriate.
> I tried changing the type of jlong to int64_t and encountered 
> compilation errors on 64 bit Linux, because I had added "#include 
> <stdint.h>" to jni_x86.h.   The header file was needed because it 
> contains the definition of int64_t.  An example error was:
>     .../src/share/vm/memory/allocation.hpp:
>          In member function void Arena::check_for_overflow(size_t, const char*) const:
>          .../src/share/vm/memory/allocation.hpp:340:
>          error: UINTPTR_MAX was not declared in this scope
> Type jlong is defined as long in the jni_md.h file that we ship on Mac 
> OS.  If we changed jlong to int64_t everywhere, then users may 
> suddenly encounter compilation errors, also.
>>> I have to admit that this change looks
>>> very clean to me.  If the type of something to print is jlong, you use
>>> JLONG_FORMAT, if it's size_t, you use SIZE_FORMAT.   This change allows
>>> macosx to compile and gets the expected result format.   I'm not sure
>>> what the purpose of having extra %xyzzy things is.  This appears to 
>>> be a
>>> clean change.
>> Clean except for the need to still special case _APPLE_. And I 
>> confess I've lost track of why that is necessary. I can see that 
>> because of the definition of int64_t on _APPLE_ you can't set 
>> JLONG_FORMAT == INT64_T_FORMAT. But I think if we have to special 
>> case something it should be the format specifier not the definition 
>> of jlong (which is what we now have). That said moving the "problem" 
>> from the jlong typedef to the JLONG_FORMAT definition isn't really 
>> any cleaner - still have an ugly _APPLE_ check in shared code. 
>> (Though all the jlong_format_specifier() removal is much cleaner.)
> The _APPLE_ check is now in 'semi' shared code, 
> globalDefintions_gcc.hpp.  That file is full of Solaris, Sparc, Linux, 
> APPLE, etc. specific code.  It appears to be an appropriate dumping 
> ground for ugly code.
>> If we go with Mikael's suggestion of using int64_t for jlong we move 
>> all the special casing to Windows. But that's because the compiler 
>> there does not support C99.
> My view of the problem reported by this bug is that the typedef for 
> jlong on Mac OS should be changed to match the other 64-bit 
> platforms.  This fix does this.  Whether or not int64_t is a better 
> definition for jlong is a different issue.  I can file an RFE to 
> investigate this.
>> David
>> -----
>>> Thanks,
>>> Coleen
>>> On 01/15/2013 05:57 PM, Mikael Vidstedt wrote:
>>>> On 1/15/2013 11:02 AM, harold seigel wrote:
>>>>> Hi,
>>>>> Thank you for the comments.  I think there are two remaining minor
>>>>> issues.  Let me know if I missed anything.
>>>>> 1. Use int64_t, instead of long, to define jlong?
>>>>>     I prefer using 'long' to define 'jlong', rather than 'int64_t',
>>>>>     because 'long' is a predefined C++ language type. Type 'int64_t'
>>>>>     is a Unix operating system defined type.  This would
>>>>>     unnecessarily complicate things.  For example, defining 'jlong'
>>>>>     as 'int64_t' would require moving the definition of 'jlong' from
>>>>>     src/cpu/x86/vm/jni_x86.h to files in the src/os_cpu/ directories.
>>>>>     Would it be useful to file a new bug to investigate using
>>>>>     'int64_t' to define 'jlong' ?
>>>> int64_t is part of the c99 standard, so it's not really an operating
>>>> system defined type per se, but I believe you're right in the sense
>>>> that it's not available in any of the standard header files on
>>>> Windows. But as I said I don't really have a problem defining jlong
>>>> based on long/long long if that's easier.
>>>> I do think it'd be a useful exercise to see what it would take to use
>>>> int64_t to define jlong, but I'm fine with doing it as a separate 
>>>> project.
>>>>> 2. Define 32-bit and 64-bit variants of JLONG_FORMAT in
>>>>> src/os/posix/launcher/java_md.h ?
>>>>>      Would it be better to define JLONG_FORMAT as %lld for 32-bit and
>>>>>     %ld for 64-bit for the posix variant, in file java_md.h?  I'm
>>>>>     unclear what the Windows variant of "%I64d" would be.
>>>> Maybe I'm missing something, but I'd say we should define jlong to be
>>>> the exact same (derived) type as int64_t, and JLONG_FORMAT should be
>>>> exactly the same as INT64_FORMAT/PRId64. For all the posix platforms I
>>>> think that should be trivial, and I'd even argue that the easiest way
>>>> to do it would be to use int64_t/PRId64 directly assuming all the
>>>> posix platforms we support have stdint.h/inttypes.h. For Windows,
>>>> judging from globalDefinitions_visCPP.hpp, it looks like "signed
>>>> __int64" and "%I64d" is the way to go regardless of 32/64. Does that
>>>> make sense?
>>>> Cheers,
>>>> Mikael
>>>>> Thanks, Harold
>>>>> On 1/14/2013 2:10 PM, Mikael Vidstedt wrote:
>>>>>> On 2013-01-12 15:05, David Holmes wrote:
>>>>>>> Sorry Harold I didn't see this before my other reply. Now I
>>>>>>> understand your problem. We either have to:
>>>>>>> a) typedef long long jlong on all platforms; or
>>>>>>> b) special case the typedef for jlong on Apple; or
>>>>>>> c) special case the typedef for JLONG_FORMAT on Apple
>>>>>>> But even if we do (a) any platform that defines int64_t differently
>>>>>>> to our jlong will mean we still need distinct format specifiers.
>>>>>>> Further unless we know how int64_t is defined on a given platform
>>>>>>> we don't know what format specifier to use (%ld or %lld).
>>>>>>> Do compilers that provide things like int64_t also define a format
>>>>>>> specifier macro?
>>>>>> It's part of the C99 standard (not sure about c++) - the types have
>>>>>> corresponding format specifier macros called PRI*, defined in
>>>>>> inttypes.h. For example PRId64, PRIu64 or PRIx64 can be used to
>>>>>> print the int64_t/uint64_t equivalents of %d, %u and %x
>>>>>> respectively. Our own internal format macros (SIZE_FORMAT,
>>>>>> INTPTR_FORMAT etc) are defined as derivatives of these.
>>>>>> In general "long" tends to be a mess... :(
>>>>>> /Mikael
>>>>>>> David
>>>>>>> -----
>>>>>>> On 12/01/2013 12:36 AM, harold seigel wrote:
>>>>>>>> Hi Vladimir,
>>>>>>>> Thank you for your comments.  Mac OS defines int64_t as 'long 
>>>>>>>> long'.
>>>>>>>> So, int64_t needs a different format specifier than jlong, 
>>>>>>>> which this
>>>>>>>> fix now defines as 'long'.  This is because, as shown below, the
>>>>>>>> Mac OS
>>>>>>>> C++ compiler is picky about format specifiers for values of types
>>>>>>>> 'long
>>>>>>>> long' and 'long'.
>>>>>>>>     $ gcc lld.cpp
>>>>>>>>     lld.cpp: In function int main(int, char**):
>>>>>>>>     lld.cpp:8: warning: format %lld expects type long long int, 
>>>>>>>> but
>>>>>>>>     argument 2 has type long int
>>>>>>>>     lld.cpp:9: warning: format %ld expects type long int, but
>>>>>>>> argument 2
>>>>>>>>     has type int64_t
>>>>>>>>     $ cat lld.cpp
>>>>>>>>     #include <stdio.h>
>>>>>>>>     #include <stdint.h>
>>>>>>>>     int main(int argc, char * argv[]) {
>>>>>>>>        long long_val = 5;
>>>>>>>>        int64_t int64_val = 8;
>>>>>>>>        printf("long_val: %ld\n", long_val);
>>>>>>>>        printf("long_val: %lld\n", long_val); <---- Line 8
>>>>>>>>        printf("int64_val: %ld\n", int64_val); <--- Line 9
>>>>>>>>        printf("int64_val: %lld\n", int64_val);
>>>>>>>>        return 0;
>>>>>>>>     }
>>>>>>>> That is why I added JLONG_FORMAT.
>>>>>>>> Thanks, Harold
>>>>>>>> On 1/10/2013 9:46 PM, Vladimir Kozlov wrote:
>>>>>>>>> Can we just define INT64_FORMAT as platform specific and use it
>>>>>>>>> instead of adding new JLONG_FORMAT?
>>>>>>>>> Thanks,
>>>>>>>>> Vladimir
>>>>>>>>> On 1/10/13 10:39 AM, harold seigel wrote:
>>>>>>>>>> Hi,
>>>>>>>>>> Please review the following changes to fix bug 7102489.
>>>>>>>>>> Summary:
>>>>>>>>>> The definition of type jlong differed on Mac OS from the other
>>>>>>>>>> 64 bit
>>>>>>>>>> platforms.  This fix makes it consistent.  In order to do this,
>>>>>>>>>> this fix
>>>>>>>>>> defines new macros, JLONG_FORMAT and JULONG_FORMAT, for printing
>>>>>>>>>> and
>>>>>>>>>> scanning jlongs and julongs.
>>>>>>>>>> This fix also does some cleanup.  Methods
>>>>>>>>>> jlong_format_specifier() and
>>>>>>>>>> julong_format_specifer() were removed and some format specifiers
>>>>>>>>>> were
>>>>>>>>>> replaced with appropriate macros.
>>>>>>>>>> Open webrev at
>>>>>>>>>> <>
>>>>>>>>>> Bug link at
>>>>>>>>>> Thank you,
>>>>>>>>>> Harold

-------------- next part --------------
An HTML attachment was scrubbed...

More information about the hotspot-runtime-dev mailing list