lhames accepted this revision.
lhames added a comment.
This revision is now accepted and ready to land.
LGTM, other than the change to the assert condition.
Thanks @sunho!
================
Comment at: llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h:1104-1108
+ assert(S == Scope::Local || llvm::count_if(AbsoluteSymbols,
+ [&](const Symbol *Sym) {
+ return Sym->getName() == Name;
+ }) == 0 &&
+ "Duplicate absolute symbol");
----------------
Why has the `S == Scope::Local` check been removed?
It's an odd "feature" but `LinkGraph` does permit duplicate names for local symbols, because they can crop up in real objects produced by `ld -r` and other tools.
================
Comment at: llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp:540-541
for (auto &E : B->edges()) {
- if (E.getTarget().getScope() == Scope::Local) {
+ if (E.getTarget().getScope() == Scope::Local &&
+ !E.getTarget().isAbsolute()) {
auto &TgtB = E.getTarget().getBlock();
----------------
This is a nice catch. I guess we're missing absolute symbol support test coverage.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130275/new/
https://reviews.llvm.org/D130275