[PATCH] D147310: [Orc] Add AutoRegisterCode option for DebugObjectManagerPlugin

[Stefan Gränitz via Phabricator via llvm-commits]

sgraenitz marked an inline comment as done.
sgraenitz added inline comments.


================
Comment at: llvm/lib/ExecutionEngine/Orc/TargetProcess/JITLoaderGDB.cpp:113
              })
       .release();
 }
----------------
lhames wrote:
> sgraenitz wrote:
> > @lhames I've never used these alloc actions in practice and just did copy/paste here. Do you think it's ok like this?
> This looks good, but you'll need to update `llvm/lib/ExecutionEngine/Orc/DebuggerSupportPlugin.cpp`'s use to pass the bool arg in.
> 
> DebugObjectManagerPlugin.h should probably be updated to use allocation actions in the future, but that can be done independently of this patch.
> 
> Side note, since I'm not sure it's well documented: An AllocAction is just an SPS wrapper function that returns an `SPSError`. It could be called via the regular RPC path, but by restricting the return type to SPSError it becomes possible to batch up calls on the LinkGraph instead: the memory manager will call the actions on our behalf (using the serialized argument buffer, so the memory manager doesn't care about the serialized argument types), and the memory manager knows how to interpret the return value, since it's always SPSError.
> 
> Since `llvm_orc_registerJITLoaderGDBAllocAction` and `llvm_orc_registerJITLoaderGDBWrapper` are identical we could just have one call the other, but they're also not a lot of code to duplicate either. 
Alright, thanks! Then this can stay as is for the moment. 

I included the update for DebuggerSupportPlugin, which I always thought had re-used the EPCDebugObjectRegistrar. It was meant to encapsulate the RPC interface. However, the DebuggerSupportPlugin does everything on its own.

Is it supposed to stay like that? Because then I will drop EPCDebugObjectRegistrar at some point..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147310