[PATCH] D97335: [Orc] Add JITLink debug support plugin for ELF x86-64

[Lang Hames via Phabricator via llvm-commits]

lhames accepted this revision.
lhames added a comment.
This revision is now accepted and ready to land.

In D97335#2591511 <https://reviews.llvm.org/D97335#2591511>, @sgraenitz wrote:

> Instead of creating a new interface, I just added a MemoryBufferRef parameter to Plugin::notifyMaterializing() for now. Is that acceptable? Otherwise, can you give a rough description of the API you had in mind?

I think that sounds good for now. Could you just add a note saying that it's deprecated? As soon as JITLink reaches the point where we can represent debug info in the graph we'll switch to using that to construct a new object,



================
Comment at: llvm/lib/ExecutionEngine/Orc/DebugObjectManagerPlugin.cpp:34
+
+  // We should never have more than one pending debug object.
+  ResourceKey Key = getResourceKey(MR);
----------------
sgraenitz wrote:
> lhames wrote:
> > sgraenitz wrote:
> > > This still holds. Can we have more than one resource per MR?
> > In this context a "resource" is some abstract thing that you manage. You can certainly manage more than one kind of resource on behalf of an MR (e.g. for an ObjC++ object you have allocated memory tracked by ObjectLinkingLayer, eh-frames tracked by the EHFrameRegistrationPlugin, and runtime data tracked by the platform plugin).
> > 
> > The usual idiom is to hold a `DenseMap<ResourceKey, std::vector<ResourceT>>`, where ResourceT is whatever resource type(s) the plugin is managing, which I think is what you're doing.
> > 
> > > // We should never have more than one pending debug object.
> > Is this true? JITLink supports running multiple concurrent instances -- If the plugin is to support concurrency it should be possible to have more than one debug object in-flight at a time.
> > If the plugin is to support concurrency it should be possible to have more than one debug object in-flight at a time.
> 
> Yes, but wouldn't they all have their own MaterializationResponsibilities?
> 
> My first thought was: yes, I can let modifyPassConfig() and notifyLoaded() simply act on all pending debug objects registered for the MR. I submitted the change and just realized now, that it is incorrect. Please see my other inline comment.
Oh -- You're right. Each in-flight object will have a distinct MaterializationResponsibility, so you can assume one debug object per MR.

Sorry for the confusion!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97335/new/

https://reviews.llvm.org/D97335