Patch: make block_comment work everywhere

[Gary Benson]

Hi Tom,

It's obviously a lot more complicated than I realised.  I had
my suspicions: the only assembler my port uses at the moment
is in interpreter code and a couple of stubs (call_stub and
flush_icache_stub), none of which use relocations.

I should be covered by Red Hat's contributor agreement.

Cheers,
Gary

Tom Rodriguez wrote:
> 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