RFR (S): 8008081: Print outs do not have matching arguments

[Mikael Vidstedt]

On 2/15/2013 12:08 AM, David Holmes wrote:
> On 13/02/2013 12:27 PM, Mikael Vidstedt wrote:
>> Please review the following change:
>>
>> http://cr.openjdk.java.net/~mikael/webrevs/8008081/webrev.00/webrev
>>
>> Background:
>>
>> I found a few occurrences of calls to print type functions where the
>> arguments do not match the format string. I hope that I have added the
>> right/intended arguments in the respective places but could use some
>> help to verify that that is indeed the case.
>
> In c1_FrameMap:
>
>  316   if( _num_monitors > 0) {
>  317     tty->print_cr("monitor [0]:%d | [%2d]:%d",
>  318                   in_bytes(sp_offset_for_monitor_base(0)),
>  319 in_bytes(sp_offset_for_monitor_base(_num_monitors)),
>  320                   _num_monitors);
>  321   }
>
> I think the last two args need swapping and should it be 
> _num_monitors-1 ie the "index" ? And does that make 
> sp_offset_for_monitor_base(_num_monitors) wrong ??

Agreed, they should indeed be swapped. Thanks Staffan/David for pointing 
this out!

Whether or not it should be _num_monitors-1 is indeed a good question. 
The code below uses num_spills-1 and prints things slightly differently 
if there is only one spill, so for symmetry reasons maybe this code 
should do the same thing? I'm counting on a C1 expert to provide guidance.

Meanwhile I've updated the webrev to add _num_monitors in the right place:

http://cr.openjdk.java.net/~mikael/webrevs/8008081/webrev.01/webrev/

> Everything else seems reasonable - some obvious, some less so - but 
> all 1000% better than before ;-)

Wow, that sure is a lot of % :)

Cheers,
Mikael

>
> David
> -----
>
>
>> One special case is in this webrev is jvmtiEnter.xls. Since it is used
>> to generate the jvmtiEnterTrace.cpp file and therefore is a bit less
>> obvious to review I have prepared a pair of before/after files for your
>> convenience:
>>
>> Before:
>> http://cr.openjdk.java.net/~mikael/webrevs/8008081/jvmtiEnterTrace.cpp.orig 
>>
>> After:
>> http://cr.openjdk.java.net/~mikael/webrevs/8008081/jvmtiEnterTrace.cpp.fixed 
>>
>>
>>
>> I also prepared a diff -u of before vs after:
>>
>> http://cr.openjdk.java.net/~mikael/webrevs/8008081/jvmtiEnterTrace.cpp.diff 
>>
>>
>> Passes JPRT.
>>
>> Cheers,
>> Mikael