Patch: make block_comment work everywhere

[Tom Rodriguez]

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 
signed one?

tom

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?
> 
> Cheers,
> Gary
>