Patch: make block_comment work everywhere
gbenson at redhat.com
Fri Dec 7 08:21:18 PST 2007
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.
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?
> 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
More information about the hotspot-dev