[PATCH] D136877: [ORC][JITLink] Do not claim dead symbols

[Lang Hames via Phabricator via llvm-commits]

lhames added a comment.

Thanks for continuing to dig into this, and sorry that it has taken me so long to find time to check it out. You've identified the underlying issue: "liveness" and "responsibility" being out-of-sync for these symbols. As things stand* though, we want to run `claimOrExternalizeWeakAndCommonSymbols` prior to dead stripping: The primary purpose of `claimOrExternalizeWeakAndCommonSymbols` is to externalize regular (non-late-breaking) weak defs that are absent from the materialization responsibility due to deliberate removal (because ORC decided to source the definition from some other materialization unit). Externalizing those weak defs before running dead stripping allows them, and their otherwise-unused transitive dependencies, to be dead-stripped. Auto-claiming "late-breaking" weak-defs at this point is mostly a matter of efficiency: the algorithm for "late breaking" weak defs is the same for regular weak defs**, and in the 99% case a late-breaking weak-def is in the graph because it's needed (so it would survive dead-stripping anyway).

I think ba26b5ef15dc <https://reviews.llvm.org/rGba26b5ef15dcbfc69f062b1aea6424cdb186e5b0> should fix your issue for now.

- Right now we have a single dead-stripping phase. Some of these phase-ordering issues may be easier to tackle if we could run dead-stripping on the graph at multiple points, but I haven't had time to give the idea serious consideration.

- We have no way to distinguish regular and "late-breaking" weaks at this level. We could distinguish them, and improve them performance of `claimAndExternalizeWeakAndCommonSymbols` if materialization responsibility objects were able to express non-responsibility (as opposed to absence of responsibility). In that case the `claimOrExternalizeWeakAndCommonSymbols` would only need to call defineMaterializing for the set of symbols where there's no responsibility indicator one way or another.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136877