This is an archive of the discontinued LLVM Phabricator instance.

[ORC] Decompose LazyCallThroughManager::callThroughToSymbol()
ClosedPublic

Authored by sgraenitz on Feb 24 2020, 2:48 PM.

Details

Summary

Decompose callThroughToSymbol() into findReexport(), resolveSymbol(), notifyResolved() and reportCallThroughError(). This allows derived classes to reuse the functionality while adding their own code in between. ThinLtoJIT's NotifyingCallThroughManager is an example for that. It wants to send a notification as soon as it figured out the symbol name.

Diff Detail

Event Timeline

sgraenitz created this revision.Feb 24 2020, 2:48 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 24 2020, 2:48 PM
sgraenitz marked an inline comment as done.Feb 24 2020, 2:56 PM
sgraenitz added inline comments.
llvm/lib/ExecutionEngine/Orc/LazyReexports.cpp
56

So far this error ends up in reportCallThroughError(), which reports it to the ExecutionSession and eventually returns ErrorHandlerAddr to the JITed code. Not sure this can be recovered from, but if so, it should probably get its own ErrorInfo.

lhames accepted this revision.Mar 3 2020, 4:55 PM

Style suggestion aside, LGTM. Thanks very much Stefan!

llvm/include/llvm/ExecutionEngine/Orc/LazyReexports.h
108

Could this lambda be factored out into a callThroughToSymbol method on LocalLazyCallThroughManager for consistency with NotifyingCallThroughManager?

This revision is now accepted and ready to land.Mar 3 2020, 4:55 PM
sgraenitz updated this revision to Diff 248333.Mar 4 2020, 2:58 PM

Factor out callThroughToSymbol(). Drop example use case from ThinLtoJIT. Format change set.

sgraenitz updated this revision to Diff 248337.Mar 4 2020, 3:11 PM

Remove assert(Notifiers.find(TrampolineAddr) != Notifiers.end()); from findReexport() -- NotifyResolved may have been issued from a concurrent call-through

Harbormaster failed remote builds in B48121: Diff 248333!
This revision was automatically updated to reflect the committed changes.