Patch: make block_comment work everywhere
Thomas.Rodriguez at Sun.COM
Wed Dec 5 17:27:30 PST 2007
That seems like a good idea and I think it's more straightforward in
many ways than the original. I'd certainly imagined it would be useful
to support comments in other situations though the original target was
really for nmethods. They are being used in the stubGenerator in a few
cases, and your changes would actually fix a minor issue there with the
comments only being recorded if PrintStubCode is on.
I'm not sure your change is completely correct though. There are a
couple things to think about. First, compiled code works by emitting
code into a temporary code blob with extra space and then relocating the
pieces that we actually filled up into a final code blob. This means
that if you move the comment collection from the CodeBuffer to the
CodeBlob you still need to transfer the comments from the temporary code
blob to the final nmethod. So the place where you deleted the
set_comments call that transfers ownership should be changed to transfer
the comments from the source blob to the dest blob. Actually you may
want to move that code into relocate_code_to so that CodeBlob::expand
will properly transfer the comments too. However you do it I think both
expand and copy_code_to should change the ownership of the comments.
The other part is that there are multiple sections in a code buffer
which can contain code and the offsets are all relative to the start of
the section during construction, with SECT_INSTS always being first.
This means that offsets into SECT_INSTS don't change after relocation
but offsets into other sections can. So if you wanted to make this
completely work you might need to add some tagging to the offsets and a
relocation pass to clean them up. Alternatively you could just
distinguish between the cases where relocations were needed and assert
that block comments don't work in those cases. All the cases where we
are generating directly code into BufferBlobs should only have one
section anyway. The existing code probably should have asserted when it
did nothing instead of silently dropping the comments.
I don't see you on the contributor agreement signatories list. Have you
Gary Benson wrote:
> Hi all,
> I've been writing the disassembler for my PPC stuff and I noticed that
> while block_comment worked in some places (stubs) it did not work in
> others (the interpreter). Looking into it I found that there are two
> places where comment lists are maintained: in CodeBlobs, and in the
> CodeBuffers that are created from them. The assembler stores comments
> in its CodeBuffer, but the disassembler looks for them in the parent
> CodeBlob. block_comment works in some places because those places
> copy the comments over to the CodeBlob.
> It seemed a little strange to be storing in one place and looking
> in another, so rather than add something to copy the comments in the
> interpreter generator I altered the assembler to store directly into
> the CodeBlob, and stripped the comment code from CodeBuffer.
> Does the attached patch look ok?
More information about the hotspot-dev