[PATCH] D57203: [ThinLTO] Refine reachability check to fix compile time increase

[Xin Tong via Phabricator via llvm-commits]

trentxintong added inline comments.


================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:830
+        visit(Index.getValueInfo(AS->getAliaseeGUID()));
+        continue;
+      }
----------------
mehdi_amini wrote:
> trentxintong wrote:
> > mehdi_amini wrote:
> > > I'm wondering about the `continue` here, the alias itself won't go through `Summary->setLive(true);` a few lines below, does it matter? 
> > > (I don't know if aliases can't have refs() as well, but it'll also skip the loop)
> > My understanding is that the alias has already been set alive when it reaches here, i.e. only live GVs are pushed into the Worklist.
> > 
> > Thank you for improving this @tejohnson.
> Then what is ` Summary->setLive(true);` line for? It only applies to Summary coming from the worklist as well? It just seems strange that this is specific to aliases.
@mehdi_amini  Good question !. AFAIK, we need this setLive(true) in case of an alias (before this patch), because if we do not, we could make the alias alive but not the aliasee. And the thinlto backend will drop dead symbols. However, it does not seem this Summary->setLive(true) is necessary here because aliasee would have been set alive in the visit() function.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57203