RFR(L): 8037816 : Fix for 8036122 breaks build with Xcode5/clang
vladimir.kozlov at oracle.com
Wed Apr 30 17:37:29 UTC 2014
On 4/30/14 4:47 AM, David Chase wrote:
> On 2014-04-30, at 2:14 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>> Thank you for fixing vm_version files.
>> In c1_LIR.cpp, c1_Runtime1.cpp, ciInstanceKlass.cpp, ciMetadata.cpp, ciObject.cpp not all 0x%" PRIxPTR were replaced by PTR_FORMAT.
>> I see you kept a lot of '0x" PRIxPTR' instead of using INTPTR_FORMAT (or PTR_FORMAT) when printing (p2i) pointers. Why?
>> Files c1_LIR.cpp, c1_Runtime1.cpp, ciInstanceKlass.cpp, ciMetadata.cpp, ciObject.cpp, markOop.cpp.
> I'm reluctant to do that, since it changes the output, from 0xbeef to 0x0000beef or 0x000000000000beef.
> Is that really acceptable?
We are printing pointers, we do and should, I think, print all their bits. It is indication that a value in an output is
a pointer. When we have bad bits I prefer to have all 0s shown: 0x00100001 instead of 0x100001. And bad pattern is
usually repeated: 0xbeefbeef.
>> memnode.cpp - for printing intptr_t values we should use INTX_FORMAT.
> got that -- this is as opposed to PRIdPTR, right?
Yes, because INTX_FORMAT is exactly "%" PRIdPTR.
Note, intx is 32 bit in 32-bit VM and 64 bit in 64-bit VM, the same as intptr_t:
typedef intptr_t intx;
>> arguments.cpp - not all (unsigned int) were replaced.
> got that -- two more.
>> I see that you enable by default ATTRIBUTE_PRINTF for print() and print_cr(). Does it mean they are checked by clang and gcc? I am for that if it is true otherwise they will degrade. I remember in first review you commented it. So I missed when it was enabled.
> I enabled it because of taking to Coleen; it seemed like an extremely good idea.
Yes, I agree.
>> General question for not these changes. Do we still need PTR_FORMAT since you used p2i() for all pointer values which use INTPTR_FORMAT? Their definition are the same. Why keep both?
> I have no idea. Whoever provided the macros did not give much guidance. PTR_FORMAT is shorter.
More information about the hotspot-dev