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

Stefan Gränitz via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Feb 27 14:36:55 PST 2021


sgraenitz added inline comments.


================
Comment at: llvm/lib/ExecutionEngine/Orc/DebugObjectManagerPlugin.cpp:395
+            return Error::success();
+          });
+    }
----------------
This obviously won't work because it adds a pass for each pending object. But what would be the right way? Sending each section range to all the pending objects? That can't seem to be correct. What am I missing?


================
Comment at: llvm/lib/ExecutionEngine/Orc/DebugObjectManagerPlugin.cpp:34
+
+  // We should never have more than one pending debug object.
+  ResourceKey Key = getResourceKey(MR);
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97335



More information about the llvm-commits mailing list