[PATCH] D43690: [ThinLTO] Keep available_externally symbols live

Mon Aug 20 10:05:29 PDT 2018

vlad.tsyrklevich added a comment.

In https://reviews.llvm.org/D43690#1201965, @twoh wrote:

> Hello, I wonder if we need to keep linkonce_odr symbols live here as well. I observe a case that a vtable for template class initiated has linkonce_odr linkage and marked dead here, which results compiler crash at WholeProgramDevirt because the global variable for vtable doesn't have initializer (https://github.com/llvm-mirror/llvm/blob/master/lib/Transforms/IPO/WholeProgramDevirt.cpp#L676 assumes that GV has the initializer). Thanks!

I'm not sure why a linkonce_odr GV would be marked non-prevailing. Do you have a minimized example to look at?

================
Comment at: llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp:550
+    // EliminateAvailableExternally pass and setting them to not-live breaks
+    // downstreams users of liveness information (PR36483).
+    if (isPrevailing(VI.getGUID()) == PrevailingType::No) {
----------------
mehdi_amini wrote:
> This explanation is not clear to me. I understand that this is fixing a bug (crash), but it isn't clear as of *why* this is the right fix?
> I'm not sure why would the simple fact that a function is defined somewhere as "available eternally" would be enough to make them "live" if we don't have any reference anywhere to make them live for another reason?
I believe the logic here is actually different. https://reviews.llvm.org/D42107 changed the live root analysis to mark non-prevailing symbols as not live even if they are reachable. I changed the logic here to continue to mark available_externally symbols live only if they're reachable.

Repository:
  rL LLVM

https://reviews.llvm.org/D43690