RFR(M/S): 8048169: Change 8037816 breaks HS build on PPC64 and CPP-Interpreter platforms

[Volker Simonis]

Hi David,

thanks for looking at the change. Please find my answers inline

On Mon, Jun 30, 2014 at 9:53 AM, David Holmes <david.holmes at oracle.com> wrote:
> Hi Volker,
>
> Two minor queries:
>
> src/share/vm/interpreter/bytecodeInterpreter.cpp
>
> +                    (int)(istate->bcp() - METHOD->code_base()),
>
> Not clear that this is an int result - what type are the operands?

The operands are both 'address' (aka 'char*') so the actual result
type would be 'ptrdiff_t'. But at this place, we subtract the code
base from the current byte code pointer the result of which should be
a short integer. I therefore think the cast should be OK.

>
> +  tty->print_cr("native_mirror: " INTPTR_FORMAT, (uintptr_t)
> this->_oop_temp);
>
> uintptr_t or intptr_t ? (Yes I see the existing code has the same mismatch
> but I don't understand why.)

This code is there since the first OpenJDK checkin and as fas as I can
see it was added with 6u10.

I think it's just the name INTPTR_FORMAT which is a little misleading.
Actually INTPTR_FORMAT is defined as:

#ifdef  _LP64
#define INTPTR_FORMAT "0x%016" PRIxPTR
#define PTR_FORMAT    "0x%016" PRIxPTR
#else   // !_LP64
#define INTPTR_FORMAT "0x%08"  PRIxPTR
#define PTR_FORMAT    "0x%08"  PRIxPTR
#endif  // _LP64

in globalDefinitions.hpp and "PRIxPTR" is the specifier for printing
an "uintptr_t" (as defined in inttypes.h by C99).

Strictly speaking, I think INTPTR_FORMAT should really be defined to
PRIiPTR (or PRIdPTR) which could then be used to output signed values.
On the other hand PTR_FORMAT should be used in the code only for
printing unsigned hex values. But there are currently hundreds of
places where INTPTR_FORMAT is used for printing unsigned hex values so
that would be another XXL change.

Regards,
Volker

>
> David
> -----
>
>
> On 28/06/2014 2:20 AM, Volker Simonis wrote:
>>
>> Hi,
>>
>> this is a follow-up for change "8037816: Fix for 8036122 breaks build
>> with Xcode5/clang". It fixes the matching of format string parameter
>> types to the actual argument types for the PPC64 and CPP-Interpreter
>> files in the same way as 8037816 already did it for all the other
>> files (so mainly insertions of 'p2i()' calls and some manual casts).
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/8048169/
>> https://bugs.openjdk.java.net/browse/JDK-8048169
>>
>> This change only touches PPC64 and the two CPP-Interpreter files
>> src/share/vm/interpreter/bytecodeInterpreter.cpp and
>> src/share/vm/interpreter/bytecodeInterpreterProfiling.hpp
>>
>> Please review and sponsor.
>>
>> Thank you and best regards,
>> Volker
>>
>