This is an archive of the discontinued LLVM Phabricator instance.

[Orc] Add AutoRegisterCode option for DebugObjectManagerPlugin
ClosedPublic

Authored by sgraenitz on Mar 31 2023, 3:26 AM.

Details

Summary

Configure the plugin to automatically call the debugger rendezvous breakpoint __jit_debug_register_code() for every translation unit (enabled) or never at all (disabled). Default API and behavior remain unchanged.

If AutoRegisterCode is turned off, it's the client's own responsibility to call the rendezvous breakpoint function at an appropriate time.
Depending on the complexity of the debugger's rendezvous breakpoint implementation, this can provide significant performance improvements in cases where many debug objects are added in sequence.

Diff Detail

Unit TestsFailed

Event Timeline

sgraenitz created this revision.Mar 31 2023, 3:26 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 31 2023, 3:26 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
sgraenitz requested review of this revision.Mar 31 2023, 3:26 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 31 2023, 3:26 AM

We don't have versioning of the RPC interface, so adding the AutoRegisterCode parameter will be a breaking change. It should show up as a compiler error for clients with customized target process implementations. That makes it obvious and easy to fix (I hope).
Despite of the RPC interface change, I didn't opt for the alternative implementation of configuring the AutoRegisterCode setting only once on the executor side. I think it's better to have it as a parameter for the actual RPC call, as this allows the controller to modify the behavior dynamically.

What do you think?

llvm/lib/ExecutionEngine/Orc/TargetProcess/JITLoaderGDB.cpp
113

@lhames I've never used these alloc actions in practice and just did copy/paste here. Do you think it's ok like this?

lhames accepted this revision.Mar 31 2023, 11:42 AM

LGTM. Thanks Stefan!

llvm/lib/ExecutionEngine/Orc/TargetProcess/JITLoaderGDB.cpp
113

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.

This revision is now accepted and ready to land.Mar 31 2023, 11:42 AM
sgraenitz updated this revision to Diff 510469.Apr 3 2023, 6:05 AM

Add documentation for AutoRegisterCode parameter

sgraenitz updated this revision to Diff 510471.Apr 3 2023, 6:11 AM

Switch to the explicit ctor in lli, llvm-jitlink and LLJITWithRemoteDebugging (not adding user-facing options for now)

sgraenitz updated this revision to Diff 510475.Apr 3 2023, 6:20 AM

Update DebuggerSupportPlugin to use the new RPC interface

sgraenitz marked an inline comment as done.Apr 3 2023, 6:35 AM
sgraenitz added inline comments.
llvm/lib/ExecutionEngine/Orc/TargetProcess/JITLoaderGDB.cpp
113

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..

This revision was landed with ongoing or failed builds.Apr 3 2023, 6:38 AM
This revision was automatically updated to reflect the committed changes.
sgraenitz marked an inline comment as done.