RFR (S): add CodeComments functionality to assember stubs (original) (raw)

Vladimir Kozlov vladimir.kozlov at oracle.com
Mon Sep 24 10:29:00 PDT 2012

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-commentsinstubs/ 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-commentsinstubs/ 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 / blockcomment() 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-commentsinstubs/ 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

More information about the hotspot-compiler-dev mailing list