Page MenuHomePhabricator

[JITLink] Pass ObjectFile in NotifyLoaded() for JITEventListener support
AbandonedPublic

Authored by sgraenitz on Apr 24 2019, 6:29 AM.

Details

Summary

Quick hack to pass ObjectFiles back to the ObjectLinkingLayer so NotifyLoaded() can include them. This may be one way to support JITEventListeners with JITLink. For more info please find my post-commit comment here: https://reviews.llvm.org/D58704#1476841

Event Timeline

sgraenitz created this revision.Apr 24 2019, 6:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 24 2019, 6:29 AM
Herald added a subscriber: hiraditya. · View Herald Transcript

This looks good, but I think we should hand back ownership of the MemoryBuffer instead, and drop the ObjectFile: The MemoryBuffer start address can serve as a stable key, and the ObjectFile can be re-created from the buffer if the client requires. This will allow us to avoid any coupling between JITLink's interface and libObject's, which will make refactoring easier when we want to move JITLink on to a libObject successor (which I think we will eventually).

Ok I see, as long as the original MemoryBuffer exists, we can recreate an identical ObjectFile on the client side. Currently it gets deleted together with JITLinkContext when returning from jitLink().

After notifyResolved(), JITLinkContext deals with the Allocation and doesn't use its MemoryBuffer anymore right? Also, at this point NotifyLoaded is called currently. If so, we may:

  1. either hand out ownership with the notification (not sure that's a good idea) and delete the (potentially moved-from) MemoryBuffer pointer here;
  2. or pass it "back" to ObjectLinkingLayer (ObjectResources?) and hand out a MemBufferRef in NotifyLoaded instead (sounds more sane to me).

What do you think?

sgraenitz updated this revision to Diff 196606.Apr 25 2019, 3:24 AM

For illustration: pass ownership to ObjectLinkingLayer and hand out a MemBufferRef in NotifyLoaded

Hi Stefan. I removed these callbacks when I added ObjectLinkingLayer::Plugin in order to tidy things up temporarily, but I think they’re still useful. I would be inclined to add two callbacks back in:

(1) NotifyLoaded — To be called as soon as we load an object (mostly useful for debugging)
(2) NotifyEmitted — To be called after we’ve fixed up and locked down the allocated memory, but before we call emit on the MaterializationResponsibility.

I would hand back ownership in the latter. Some clients like to be able to hold on to the buffer after the JIT is done with it.

Ok, I went with MCJIT for now. Will leave this here for future reference.

sgraenitz abandoned this revision.May 9 2019, 2:34 AM