RFR (S): add CodeComments functionality to assember stubs

[Vladimir Kozlov]

Thank you, Goetz

Lindenmaier, Goetz wrote:
> Hi Vladimir,
> 
> Sorry for the trouble, I didn't know about jcheck.  I also prefer code without

No problems.

> needless invisible stuff, so I'm happy to use it.
> I fixed the whitespace.  Nevertheless I can't get it 
> through jcheck clean, as I don't have the proper user, bugid etc.
> 
> I still get: 
>    Invalid changeset author: Goetz Lindenmaier

This field uses openjdk id and you have one: goetz
Unfortunately I can't use it for these changes in Hotspot since you are not, 
yet, Author for HSX (Hotspot) project. You can be nominated after few 
contributions if you want.

>    Incomplete comment: Missing bugid line

I included you in interest list when I created bug you should known bug id.

>    Incomplete comment: Missing reviewer attribution
>    Extraneous text in comment

Here is changeset comment I will use:

7200163: add CodeComments functionality to assember stubs
Summary: Pass the codeBuffer to the Stub constructor, and adapts the 
disassembler to print the comments.
Reviewed-by: jrose, kvn, twisti
Contributed-by: Goetz Lindenmaier

> I hope this is ok.

Yes, it is definitely ok. With latest changes commit instruction passed without 
problems. I will submit job to push it.

Regards,
Vladimir

> 
> The fixed webrev is again at 
> http://cr.openjdk.java.net/~goetz/webrevs/webrev-comments_in_stubs/
> 
> Thanks for your patience,
>   Goetz.
> 
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com] 
> Sent: Friday, September 21, 2012 6:34 PM
> To: Lindenmaier, Goetz
> Cc: Christian Thalinger; hotspot-compiler-dev at openjdk.java.net
> Subject: Re: RFR (S): add CodeComments functionality to assember stubs
> 
> Hi Goetz,
> 
> We require that code changes do not have tabs (\t), trailing spaces and use unix 
> new lines (no CR at the line end). We have jcheck routine which verifies that. 
> Your changes have problems:
> 
> src/share/vm/code/icBuffer.hpp:52: Trailing whitespace
> src/share/vm/code/stubs.hpp:75: Trailing whitespace
> src/share/vm/interpreter/interpreter.hpp:55: Trailing whitespace
> src/share/vm/runtime/sharedRuntime.cpp:2474: Trailing whitespace
> 
> Here is description how you can install and use the tool:
> 
> http://openjdk.java.net/projects/code-tools/jcheck/
> 
> Thanks,
> Vladimir
> 
> Lindenmaier, Goetz wrote:
>> Hi,
>>
>> I fixed the line feeds.  The new webrev is at the same address: 
>> http://cr.openjdk.java.net/~goetz/webrevs/webrev-comments_in_stubs/
>>
>> Thanks a lot for reviewing and the positive comments!
>>   Goetz
>>
>>
>>
>> -----Original Message-----
>> From: Christian Thalinger [mailto:christian.thalinger at oracle.com] 
>> Sent: Freitag, 21. September 2012 02:43
>> To: Lindenmaier, Goetz
>> Cc: hotspot-compiler-dev at openjdk.java.net
>> Subject: Re: RFR (S): add CodeComments functionality to assember stubs
>>
>>
>> On Sep 19, 2012, at 5:20 AM, "Lindenmaier, Goetz" <goetz.lindenmaier at sap.com> wrote:
>>
>>> Hi,
>>>  
>>> The Assembler and CodeBuffer classes supply CodeComment / block_comment()
>>> functionality, which does not work with stubs. The comments are
>>> not printed with +PrintStubCode or +PrintInterpreter because the
>>> comments are lost when the code is turned into a Stub, while
>>> they are kept if the code is copied to a CodeBlob.
>>>  
>>> We fixed this in our SAP JVM, and contributed the change to the
>>> ppc-aix-port some while ago, see
>>> http://hg.openjdk.java.net/ppc-aix-port/jdk7u/hotspot/rev/d65d0876ab43.
>>>  
>>> I propose to add this fix to the OpenJDK mainline.
>>> A webrev can be found here:
>>> http://cr.openjdk.java.net/~goetz/webrevs/webrev-comments_in_stubs/
>>>  
>>> Basically the change passes the codeBuffer to the Stub 
>>> constructor, and adapts the disassembler to print the
>>> comments.
>>> In the debug build the  InterpreterCodelet Stub has a new field
>>> holding the code comments.
>>>  
>>> I also added some ttyLocks and \\ns to beautify the output.
>> +      tty->print("\n");
>>
>> Can you replace these with:
>>
>> +      tty->cr();
>>
>> Otherwise this looks good and we should integrate it.  Thanks for contributing!
>>
>> -- Chris
>>
>>>  
>>> Could somebody please create a bug id for this issue and review the changes?
>>>  
>>> Thank you and best regards,
>>>   Goetz