From what I can tell, the loop inside applyExternalSymbolRelocations()
used to call getSymbolAddress(). After the JITSymbolResolver interface
redesign, the functionality has changed, and the loop should no longer
trigger repopulation of ExternalSymbolRelocations. If that's the case,
there is no need to update the loop iterator manually, and
ExternalSymbolRelocations can be cleared at once. This way, when there
are many external symbols in the program, the function runs much faster.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
@lhames: does this diff make sense to you or I missed a way ExternalSymbolRelocations can get altered during the loop execution?
After the JITSymbolResolver interface redesign, the functionality has changed, and the loop should no longer trigger repopulation of ExternalSymbolRelocations.
That API change happened a while ago so my recollection may be wrong, but I don't think it changed the fundamental fact that lookup could trigger linking of additional modules. If that's the case I think we could still end up with additional externals needing to be resolved.
Side note: this issue is fixed in ORCv2 / JITLink.
llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp | ||
---|---|---|
1140 | That (triggering linking of additional modules) is accomplished here on this loop inside RuntimeDyldImpl::resolveExternalSymbols() It doesn't look like applyExternalSymbolRelocations() (the previous function) needs to expect that ExternalSymbolMap will be changed while it is iterating over it (see the call in line 1188 happening only after all modules have been loaded). |
Oh, that getSymbolAddress call was moved out of the loop. Ok -- Looks good to me.
Thanks for this Maksim, and for the clarification Rafael.
- Lang.
That (triggering linking of additional modules) is accomplished here on this loop inside RuntimeDyldImpl::resolveExternalSymbols()
It doesn't look like applyExternalSymbolRelocations() (the previous function) needs to expect that ExternalSymbolMap will be changed while it is iterating over it (see the call in line 1188 happening only after all modules have been loaded).