[ThinLTO] Handle chains of aliases

[tejohnson]

Summary

At -O0, globalopt is not run during the compile step, and we can have a
chain of an alias having an immediate aliasee of another alias. We were
not correctly summarizing this (we always got the base object when
recording the aliasee), and also not handling it properly in ThinLTO
dead stripping, which would only mark the base object as live. This
could result in the intermediate alias incorrectly being marked dead and
dropped to a declaration.

Fix by correctly representing an alias to another alias in the summary.
Also change the dead stripping computation so that it marks all aliases
in the chain as live.

This required adding a version of stripInBoundsOffsets() that doesn't
follow aliases (this is the interface called by getBaseObject, and we
want to be able to strip in bounds offsets from the immediate aliasee).

[tejohnson]

ping

[tejohnson]

ping

[davidxl]

include/llvm/IR/ModuleSummaryIndex.h

427

Can this be refactored as

return const_cast<const GlobalValueSummary *>(this)->getBaseObject();

to avoid code duplication?

lib/Analysis/ModuleSummaryAnalysis.cpp

446

Add a comment for the function?

lib/IR/Value.cpp

475

Add some documentation on the InBounds kinds?

[tejohnson]

include/llvm/IR/ModuleSummaryIndex.h

427

Yep. Although needs another const_cast on the return value to remove the const again.

[tejohnson]

Address comments

[pcc]

Isn't this just a bug in promotion? It should probably internalize any non-prevailing aliases like lib/Linker does.

[davidxl]

lib/IR/Value.cpp

475

Perhaps there is no need to change the name of this kind -- proper documentation is probably enough.

476

Perhaps leave the semantics of this enum unchanged?

477

Name the new one PSK_InBoundsNoAliases so that existing enumerator's semantics do not change.

[tejohnson]

In D54507#1327394, @pcc wrote:

Isn't this just a bug in promotion? It should probably internalize any non-prevailing aliases like lib/Linker does.

Can you clarify? The summary doesn't contain any references to the intermediate aliasee so it looks completely unreferenced.

[pcc]

In D54507#1327487, @tejohnson wrote:

In D54507#1327394, @pcc wrote:

Isn't this just a bug in promotion? It should probably internalize any non-prevailing aliases like lib/Linker does.

Can you clarify? The summary doesn't contain any references to the intermediate aliasee so it looks completely unreferenced.

And indeed it isn't. The important thing to keep alive is the base object, which is what the code already does.

My first idea was that we would change the code in convertToDeclaration here:
http://llvm-cs.pcc.me.uk/lib/Transforms/IPO/FunctionImport.cpp#923
so that we would RAUW all alias references to point to the external and all non-alias references to point to the aliasee. But unfortunately it doesn't seem easy to implement "replace some uses with" on constants.

Probably the simplest fix is to make sure that each aliasee is canonicalized to point to the base object before calling convertToDeclaration. This seems pretty straightforward using ConstantExpr::getWithOperands. This also takes us one step closer to the canonical alias form we discussed last year: we would just need to apply the same canonicalization *before* writing the bitcode file.

[tejohnson]

In D54507#1327573, @pcc wrote:

In D54507#1327487, @tejohnson wrote:

In D54507#1327394, @pcc wrote:

Isn't this just a bug in promotion? It should probably internalize any non-prevailing aliases like lib/Linker does.

Can you clarify? The summary doesn't contain any references to the intermediate aliasee so it looks completely unreferenced.

And indeed it isn't. The important thing to keep alive is the base object, which is what the code already does.

My first idea was that we would change the code in convertToDeclaration here:
http://llvm-cs.pcc.me.uk/lib/Transforms/IPO/FunctionImport.cpp#923
so that we would RAUW all alias references to point to the external and all non-alias references to point to the aliasee. But unfortunately it doesn't seem easy to implement "replace some uses with" on constants.

Probably the simplest fix is to make sure that each aliasee is canonicalized to point to the base object before calling convertToDeclaration.

Do you mean make sure that each "alias" is canonicalized to point to the base object? I.e. if we have an alias relationship "A aliases B aliases C", then convert it to "A aliases C" and "B aliases C"? Presumably that would fix the issue, but the summary is still not accurately reflecting the IR (although not sure where else it would be problematic).

This seems pretty straightforward using ConstantExpr::getWithOperands. This also takes us one step closer to the canonical alias form we discussed last year: we would just need to apply the same canonicalization *before* writing the bitcode file.

Yes this alias canonicalization would also fix the issue, before the summary is generated. Unfortunately it's not something I have had time to work on since that first attempt.

[pcc]

In D54507#1327609, @tejohnson wrote:

In D54507#1327573, @pcc wrote:

In D54507#1327487, @tejohnson wrote:

In D54507#1327394, @pcc wrote:

Isn't this just a bug in promotion? It should probably internalize any non-prevailing aliases like lib/Linker does.

Can you clarify? The summary doesn't contain any references to the intermediate aliasee so it looks completely unreferenced.

And indeed it isn't. The important thing to keep alive is the base object, which is what the code already does.

My first idea was that we would change the code in convertToDeclaration here:
http://llvm-cs.pcc.me.uk/lib/Transforms/IPO/FunctionImport.cpp#923
so that we would RAUW all alias references to point to the external and all non-alias references to point to the aliasee. But unfortunately it doesn't seem easy to implement "replace some uses with" on constants.

Probably the simplest fix is to make sure that each aliasee is canonicalized to point to the base object before calling convertToDeclaration.

Do you mean make sure that each "alias" is canonicalized to point to the base object? I.e. if we have an alias relationship "A aliases B aliases C", then convert it to "A aliases C" and "B aliases C"?

Yes, exactly.

Presumably that would fix the issue, but the summary is still not accurately reflecting the IR (although not sure where else it would be problematic).

I don't think it would be problematic. In this case the summary contains the more accurate representation of reality because references to globals in alias definitions, unlike all other references, are not subject to linker preemption, and we shouldn't allow the less-accurate representation in the IR to leak anywhere else.

This seems pretty straightforward using ConstantExpr::getWithOperands. This also takes us one step closer to the canonical alias form we discussed last year: we would just need to apply the same canonicalization *before* writing the bitcode file.

Yes this alias canonicalization would also fix the issue, before the summary is generated. Unfortunately it's not something I have had time to work on since that first attempt.

Yes but note that I am not proposing that we canonicalize everywhere right now. We just need to canonicalize here: http://llvm-cs.pcc.me.uk/lib/LTO/LTOBackend.cpp#490 (before dropDeadSymbols).

[tejohnson]

In D54507#1327677, @pcc wrote:

In D54507#1327609, @tejohnson wrote:

In D54507#1327573, @pcc wrote:

In D54507#1327487, @tejohnson wrote:

In D54507#1327394, @pcc wrote:

Isn't this just a bug in promotion? It should probably internalize any non-prevailing aliases like lib/Linker does.

Can you clarify? The summary doesn't contain any references to the intermediate aliasee so it looks completely unreferenced.

And indeed it isn't. The important thing to keep alive is the base object, which is what the code already does.

My first idea was that we would change the code in convertToDeclaration here:
http://llvm-cs.pcc.me.uk/lib/Transforms/IPO/FunctionImport.cpp#923
so that we would RAUW all alias references to point to the external and all non-alias references to point to the aliasee. But unfortunately it doesn't seem easy to implement "replace some uses with" on constants.

Probably the simplest fix is to make sure that each aliasee is canonicalized to point to the base object before calling convertToDeclaration.

Do you mean make sure that each "alias" is canonicalized to point to the base object? I.e. if we have an alias relationship "A aliases B aliases C", then convert it to "A aliases C" and "B aliases C"?

Yes, exactly.

Presumably that would fix the issue, but the summary is still not accurately reflecting the IR (although not sure where else it would be problematic).

I don't think it would be problematic. In this case the summary contains the more accurate representation of reality because references to globals in alias definitions, unlike all other references, are not subject to linker preemption, and we shouldn't allow the less-accurate representation in the IR to leak anywhere else.

Philosophically, I think the summary should summarize the IR that we have (in the bitcode file), not the IR that we want. So I'm not completely comfortable with this approach - I'm actually thinking it may be better to add a skeleton alias canonicalization pass (taken from that draft alias canonicalization patch D29781) that is invoked during thinlto preparation. For now, it can just do this flatting of alias chains, and eventually expanded into the full canonicalization (and invoked in all paths as described in that patch description). I don't want to sign up to do the whole hog canonicalization currently since this bug only shows up at -O0 and I don't have the bandwidth to focus on it ATM. Does that seem like a reasonable interim approach?

This seems pretty straightforward using ConstantExpr::getWithOperands. This also takes us one step closer to the canonical alias form we discussed last year: we would just need to apply the same canonicalization *before* writing the bitcode file.

Yes this alias canonicalization would also fix the issue, before the summary is generated. Unfortunately it's not something I have had time to work on since that first attempt.

Yes but note that I am not proposing that we canonicalize everywhere right now. We just need to canonicalize here: http://llvm-cs.pcc.me.uk/lib/LTO/LTOBackend.cpp#490 (before dropDeadSymbols).

[pcc]

In D54507#1328447, @tejohnson wrote:

In D54507#1327677, @pcc wrote:

In D54507#1327609, @tejohnson wrote:

In D54507#1327573, @pcc wrote:

In D54507#1327487, @tejohnson wrote:

In D54507#1327394, @pcc wrote:

Isn't this just a bug in promotion? It should probably internalize any non-prevailing aliases like lib/Linker does.

Can you clarify? The summary doesn't contain any references to the intermediate aliasee so it looks completely unreferenced.

And indeed it isn't. The important thing to keep alive is the base object, which is what the code already does.

My first idea was that we would change the code in convertToDeclaration here:
http://llvm-cs.pcc.me.uk/lib/Transforms/IPO/FunctionImport.cpp#923
so that we would RAUW all alias references to point to the external and all non-alias references to point to the aliasee. But unfortunately it doesn't seem easy to implement "replace some uses with" on constants.

Probably the simplest fix is to make sure that each aliasee is canonicalized to point to the base object before calling convertToDeclaration.

Do you mean make sure that each "alias" is canonicalized to point to the base object? I.e. if we have an alias relationship "A aliases B aliases C", then convert it to "A aliases C" and "B aliases C"?

Yes, exactly.

Presumably that would fix the issue, but the summary is still not accurately reflecting the IR (although not sure where else it would be problematic).

I don't think it would be problematic. In this case the summary contains the more accurate representation of reality because references to globals in alias definitions, unlike all other references, are not subject to linker preemption, and we shouldn't allow the less-accurate representation in the IR to leak anywhere else.

Philosophically, I think the summary should summarize the IR that we have (in the bitcode file), not the IR that we want. So I'm not completely comfortable with this approach - I'm actually thinking it may be better to add a skeleton alias canonicalization pass (taken from that draft alias canonicalization patch D29781) that is invoked during thinlto preparation. For now, it can just do this flatting of alias chains, and eventually expanded into the full canonicalization (and invoked in all paths as described in that patch description). I don't want to sign up to do the whole hog canonicalization currently since this bug only shows up at -O0 and I don't have the bandwidth to focus on it ATM. Does that seem like a reasonable interim approach?

Sounds reasonable enough. It seems a little better to do this during LTO because it would allow us to upgrade old bitcode but I'm happy for it to be a pass as well. I left some comments on D29781 about the overall approach there.

[tejohnson]

In D54507#1328732, @pcc wrote:

In D54507#1328447, @tejohnson wrote:

In D54507#1327677, @pcc wrote:

In D54507#1327609, @tejohnson wrote:

In D54507#1327573, @pcc wrote:

In D54507#1327487, @tejohnson wrote:

In D54507#1327394, @pcc wrote:

Isn't this just a bug in promotion? It should probably internalize any non-prevailing aliases like lib/Linker does.

Can you clarify? The summary doesn't contain any references to the intermediate aliasee so it looks completely unreferenced.

And indeed it isn't. The important thing to keep alive is the base object, which is what the code already does.

My first idea was that we would change the code in convertToDeclaration here:
http://llvm-cs.pcc.me.uk/lib/Transforms/IPO/FunctionImport.cpp#923
so that we would RAUW all alias references to point to the external and all non-alias references to point to the aliasee. But unfortunately it doesn't seem easy to implement "replace some uses with" on constants.

Probably the simplest fix is to make sure that each aliasee is canonicalized to point to the base object before calling convertToDeclaration.

Do you mean make sure that each "alias" is canonicalized to point to the base object? I.e. if we have an alias relationship "A aliases B aliases C", then convert it to "A aliases C" and "B aliases C"?

Yes, exactly.

Presumably that would fix the issue, but the summary is still not accurately reflecting the IR (although not sure where else it would be problematic).

I don't think it would be problematic. In this case the summary contains the more accurate representation of reality because references to globals in alias definitions, unlike all other references, are not subject to linker preemption, and we shouldn't allow the less-accurate representation in the IR to leak anywhere else.

Philosophically, I think the summary should summarize the IR that we have (in the bitcode file), not the IR that we want. So I'm not completely comfortable with this approach - I'm actually thinking it may be better to add a skeleton alias canonicalization pass (taken from that draft alias canonicalization patch D29781) that is invoked during thinlto preparation. For now, it can just do this flatting of alias chains, and eventually expanded into the full canonicalization (and invoked in all paths as described in that patch description). I don't want to sign up to do the whole hog canonicalization currently since this bug only shows up at -O0 and I don't have the bandwidth to focus on it ATM. Does that seem like a reasonable interim approach?

Sounds reasonable enough. It seems a little better to do this during LTO because it would allow us to upgrade old bitcode but I'm happy for it to be a pass as well. I left some comments on D29781 about the overall approach there.

Uploading patch to do this now. Will update the summary if we go with this approach. Note that the clang side patch which I will upload separately contains the invocations of the new pass for the new PM (mirroring how this was done for NameAnonGlobals).

[tejohnson]

Update per comments.

[pcc]

lib/Transforms/Utils/CanonicalizeAliases.cpp

51 ↗

(On Diff #177918)

This isn't quite right; it will rewrite

ga1 = alias ga2
ga2 = alias gep(gv, 0, 1)

as

ga1 = alias gv

i.e. you will drop the offset from the alias. The implementation in https://reviews.llvm.org/D29781#1328720 handles this case correctly.

[tejohnson]

lib/Transforms/Utils/CanonicalizeAliases.cpp

51 ↗

(On Diff #177918)

You're right. We want to look at the direct aliasee (recursively). Made that change and enhanced the test case to cover this.

[tejohnson]

Address comments

[tejohnson]

lib/Transforms/Utils/CanonicalizeAliases.cpp

51 ↗

(On Diff #177918)

Actually, I think my new approach won't quite work if both aliases in the chain involve expressions - probably needs something like your ConstantExpr case in D29781. Need to construct an example.

[pcc]

lib/Transforms/Utils/CanonicalizeAliases.cpp

40 ↗

(On Diff #177983)

I think you can change this to namespace { and then the functions below don't need to be static.

48 ↗

(On Diff #177983)

With the current code you don't actually need the recursive call here, it could just be

while (auto *GA2 = dyn_cast<GlobalAlias>(Aliasee))
  Aliasee = GA2->getAliasee();

(but ignore if you plan to make this recursive).

51 ↗

(On Diff #177983)

This is just GA->setAliasee(Aliasee), no?

51 ↗

(On Diff #177918)

Yes, here's an example that won't canonicalize correctly:

ga1 = alias gep(ga2, 0, 1)
ga2 = alias gv

[tejohnson]

New version which more closely mirrors logic proposed in D29781 comments. One difference is that I update all aliases in the chain in one recursive pass. In any case, this should make it easy to implement D29781 on top as a follow on.

lib/Transforms/Utils/CanonicalizeAliases.cpp

48 ↗

(On Diff #177983)

I decided to make it recursive, so we can fix up all aliases in the chain in one go.

51 ↗

(On Diff #177983)

Right, that was leftover from an older flawed version.

51 ↗

(On Diff #177918)

Added that case to my test as well.

[tejohnson]

Fix algorithm, update test.

[pcc]

lib/Transforms/Utils/CanonicalizeAliases.cpp

61 ↗

(On Diff #178074)

You don't need to track this yourself, getWithOperands will do it for you.
http://llvm-cs.pcc.me.uk/lib/IR/Constants.cpp#1185

70 ↗

(On Diff #178074)

Remove braces

[tejohnson]

Address comments

[tejohnson]

ping - @pcc does this look ok now?

[pcc]

LGTM

lib/Transforms/IPO/PassManagerBuilder.cpp

749 ↗

(On Diff #178280)

Move comment.

[tejohnson]

Address comment

include/llvm/IR/ModuleSummaryIndex.h

public:
   bool hasAliaseeGUID() const { return AliaseeGUID != 0; }
   const GlobalValue::GUID &getAliaseeGUID() const {
     assert(AliaseeGUID && "Unexpected missing aliasee GUID");
     return AliaseeGUID;
   }
 };
 
 const inline GlobalValueSummary *GlobalValueSummary::getBaseObject() const {
-  if (auto *AS = dyn_cast<AliasSummary>(this))
-    return &AS->getAliasee();
-  return this;
+  auto *S = this;
+  while (auto *AS = dyn_cast<AliasSummary>(S))
+    S = &AS->getAliasee();
+  return S;
 }
 
 inline GlobalValueSummary *GlobalValueSummary::getBaseObject() {
-  if (auto *AS = dyn_cast<AliasSummary>(this))
-    return &AS->getAliasee();
-  return this;
+  auto *S = this;
+  while (auto *AS = dyn_cast<AliasSummary>(S))
+    S = &AS->getAliasee();
+  return S;
 }
 
 /// Function summary information to aid decisions and implementation of
 /// importing.
 class FunctionSummary : public GlobalValueSummary {
 public:
   /// <CalleeValueInfo, CalleeInfo> call edge pair.
   using EdgeTy = std::pair<ValueInfo, CalleeInfo>;

include/llvm/IR/Value.h

#include "llvm/IR/Value.def"
   /// Returns the original pointer value.  If this is called on a non-pointer
   /// value, it returns 'this'.
   const Value *stripInBoundsOffsets() const;
   Value *stripInBoundsOffsets() {
     return const_cast<Value *>(
                       static_cast<const Value *>(this)->stripInBoundsOffsets());
   }
 
+  /// Strip off pointer casts and inbounds GEPs, without following aliases.
+  ///
+  /// Returns the original pointer value.  If this is called on a non-pointer
+  /// value, it returns 'this'.
+  const Value *stripInBoundsOffsetsNoFollowAliases() const;
+  Value *stripInBoundsOffsetsNoFollowAliases() {
+    return const_cast<Value *>(static_cast<const Value *>(this)
+                                   ->stripInBoundsOffsetsNoFollowAliases());
+  }
+
   /// Returns the number of bytes known to be dereferenceable for the
   /// pointer value.
   ///
   /// If CanBeNull is set by this function the pointer can either be null or be
   /// dereferenceable up to the returned number of bytes.
   uint64_t getPointerDereferenceableBytes(const DataLayout &DL,
                                           bool &CanBeNull) const;
 

lib/Analysis/ModuleSummaryAnalysis.cpp

 
 static void
 computeAliasSummary(ModuleSummaryIndex &Index, const GlobalAlias &A,
                     DenseSet<GlobalValue::GUID> &CantBePromoted) {
   bool NonRenamableLocal = isNonRenamableLocal(A);
   GlobalValueSummary::GVFlags Flags(A.getLinkage(), NonRenamableLocal,
                                     /* Live = */ false, A.isDSOLocal());
   auto AS = llvm::make_unique<AliasSummary>(Flags);
-  auto *Aliasee = A.getBaseObject();
-  auto *AliaseeSummary = Index.getGlobalValueSummary(*Aliasee);
-  assert(AliaseeSummary && "Alias expects aliasee summary to be parsed");
-  AS->setAliasee(AliaseeSummary);
   if (NonRenamableLocal)
     CantBePromoted.insert(A.getGUID());
   Index.addGlobalValueSummary(A, std::move(AS));
 }
 
+static void finalizeAliasSummary(ModuleSummaryIndex &Index,
+                                 const GlobalAlias &A) {
+  auto *AS = cast<AliasSummary>(Index.getGlobalValueSummary(A));
+  auto *Aliasee =
+      cast<GlobalValue>(A.getAliasee()->stripInBoundsOffsetsNoFollowAliases());
+  auto *AliaseeSummary = Index.getGlobalValueSummary(*Aliasee);
+  assert(AliaseeSummary && "Alias expects aliasee summary to be parsed");
+  AS->setAliasee(AliaseeSummary);
+}
+
 // Set LiveRoot flag on entries matching the given value name.
 static void setLiveRoot(ModuleSummaryIndex &Index, StringRef Name) {
   if (ValueInfo VI = Index.getValueInfo(GlobalValue::getGUID(Name)))
     for (auto &Summary : VI.getSummaryList())
       Summary->setLive(true);
 }
 
 ModuleSummaryIndex llvm::buildModuleSummaryIndex(
ModuleSummaryIndex llvm::buildModuleSummaryIndex(
   // index.
   for (const GlobalVariable &G : M.globals()) {
     if (G.isDeclaration())
       continue;
     computeVariableSummary(Index, G, CantBePromoted);
   }
 
   // Compute summaries for all aliases defined in module, and save in the
-  // index.
+  // index. In the first iteration, just create the alias summaries, but
+  // don't attempt to set the aliasee summaries, which might not be created
+  // yet if this is an alias to another alias.
   for (const GlobalAlias &A : M.aliases())
     computeAliasSummary(Index, A, CantBePromoted);
+  // Walk the aliases again and set up the aliasee summary pointers, now
+  // that all summaries have been created.
+  for (const GlobalAlias &A : M.aliases())
+    finalizeAliasSummary(Index, A);
 
   for (auto *V : LocalsUsed) {
     auto *Summary = Index.getGlobalValueSummary(*V);
     assert(Summary && "Missing summary for global value");
     Summary->setNotEligibleToImport();
   }
 
   // The linker doesn't know about these LLVM produced values, so we need

lib/Bitcode/Reader/BitcodeReader.cpp

Error ModuleSummaryIndexBitcodeReader::parseEntireSummary(unsigned ID) {
   // each function summary records; these variables store the information
   // collected so far so that it can be used to create the summary object.
   std::vector<GlobalValue::GUID> PendingTypeTests;
   std::vector<FunctionSummary::VFuncId> PendingTypeTestAssumeVCalls,
       PendingTypeCheckedLoadVCalls;
   std::vector<FunctionSummary::ConstVCall> PendingTypeTestAssumeConstVCalls,
       PendingTypeCheckedLoadConstVCalls;
 
+  // Keep track of aliases that haven't had their aliasee resolved yet
+  // (because the aliasee is a forward referenced alias).
+  std::vector<AliasSummary *> AliasesToResolve;
+
   while (true) {
     BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
 
     switch (Entry.Kind) {
     case BitstreamEntry::SubBlock: // Handled for us already.
     case BitstreamEntry::Error:
       return error("Malformed block");
-    case BitstreamEntry::EndBlock:
+    case BitstreamEntry::EndBlock: {
+      // Now that we have scanned all summaries, set up the aliasee
+      // pointers for any that weren't available earlier.
+      for (auto *AS : AliasesToResolve) {
+        auto AliaseeInModule = TheIndex.findSummaryInModule(
+            AS->getAliaseeGUID(), AS->modulePath());
+        if (!AliaseeInModule)
+          return error("Alias expects aliasee summary to be parsed");
+        AS->setAliasee(AliaseeInModule);
+      }
       return Error::success();
+    }
     case BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // Read a record. The record format depends on whether this
     // is a per-module index or a combined index file. In the per-module
     // case the records contain the associated value's ID for correlation
case bitc::FS_ALIAS: {
       // index's module string table. Since we don't have a module path
       // string table section in the per-module index, we create a single
       // module path string table entry with an empty (0) ID to take
       // ownership.
       AS->setModulePath(getThisModule()->first());
 
       GlobalValue::GUID AliaseeGUID =
           getValueInfoFromValueId(AliaseeID).first.getGUID();
+      AS->setAliaseeGUID(AliaseeGUID);
       auto AliaseeInModule =
           TheIndex.findSummaryInModule(AliaseeGUID, ModulePath);
-      if (!AliaseeInModule)
-        return error("Alias expects aliasee summary to be parsed");
+      if (AliaseeInModule)
         AS->setAliasee(AliaseeInModule);
-      AS->setAliaseeGUID(AliaseeGUID);
+      else
+        // Must be a forward reference to another alias, record and set up
+        // aliasee pointer later.
+        AliasesToResolve.push_back(AS.get());
 
       auto GUID = getValueInfoFromValueId(ValueID);
       AS->setOriginalName(GUID.second);
       TheIndex.addGlobalValueSummary(GUID.first, std::move(AS));
       break;
     }
     // FS_PERMODULE_GLOBALVAR_INIT_REFS: [valueid, flags, varflags, n x valueid]
     case bitc::FS_PERMODULE_GLOBALVAR_INIT_REFS: {

lib/Bitcode/Writer/BitcodeWriter.cpp

void ModuleBitcodeWriterBase::writePerModuleGlobalValueSummary() {
   }
 
   // Capture references from GlobalVariable initializers, which are outside
   // of a function scope.
   for (const GlobalVariable &G : M.globals())
     writeModuleLevelReferences(G, NameVals, FSModRefsAbbrev);
 
   for (const GlobalAlias &A : M.aliases()) {
-    auto *Aliasee = A.getBaseObject();
-    if (!Aliasee->hasName())
-      // Nameless function don't have an entry in the summary, skip it.
-      continue;
+    auto *Aliasee = cast<const GlobalValue>(
+        A.getAliasee()->stripInBoundsOffsetsNoFollowAliases());
+    assert(Aliasee->hasName());
     auto AliasId = VE.getValueID(&A);
     auto AliaseeId = VE.getValueID(Aliasee);
     NameVals.push_back(AliasId);
     auto *Summary = Index->getGlobalValueSummary(A);
     AliasSummary *AS = cast<AliasSummary>(Summary);
+#ifndef NDEBUG
+    // Ensure that the aliasee summary pointer in the alias summary
+    // is the same as the summary obtained from the aliasee GV.
+    ValueInfo AliaseeVI = Index->getValueInfo(Aliasee->getGUID());
+    auto *AliaseeSummary = AliaseeVI.getSummaryList()[0].get();
+    assert(AliaseeSummary == &AS->getAliasee());
+#endif
     NameVals.push_back(getEncodedGVSummaryFlags(AS->flags()));
     NameVals.push_back(AliaseeId);
     Stream.EmitRecord(bitc::FS_ALIAS, NameVals, FSAliasAbbrev);
     NameVals.clear();
   }
 
   Stream.ExitBlock();
 }

lib/IR/Value.cpp

 }
 
 namespace {
 // Various metrics for how much to strip off of pointers.
 enum PointerStripKind {
   PSK_ZeroIndices,
   PSK_ZeroIndicesAndAliases,
   PSK_ZeroIndicesAndAliasesAndInvariantGroups,
-  PSK_InBoundsConstantIndices,
+  PSK_InBoundsConstantIndicesAndAliases,
+  PSK_InBoundsAndAliases,
   PSK_InBounds
 };
 
 template <PointerStripKind StripKind>
 static const Value *stripPointerCastsAndOffsets(const Value *V) {
   if (!V->getType()->isPointerTy())
     return V;
 
   // Even though we don't look through PHI nodes, we could be called on an
   // instruction in an unreachable block, which may be on a cycle.
   SmallPtrSet<const Value *, 4> Visited;
 
   Visited.insert(V);
   do {
     if (auto *GEP = dyn_cast<GEPOperator>(V)) {
       switch (StripKind) {
       case PSK_ZeroIndicesAndAliases:
       case PSK_ZeroIndicesAndAliasesAndInvariantGroups:
       case PSK_ZeroIndices:
         if (!GEP->hasAllZeroIndices())
           return V;
         break;
-      case PSK_InBoundsConstantIndices:
+      case PSK_InBoundsConstantIndicesAndAliases:
         if (!GEP->hasAllConstantIndices())
           return V;
         LLVM_FALLTHROUGH;
+      case PSK_InBoundsAndAliases:
       case PSK_InBounds:
         if (!GEP->isInBounds())
           return V;
         break;
       }
       V = GEP->getPointerOperand();
     } else if (Operator::getOpcode(V) == Instruction::BitCast ||
                Operator::getOpcode(V) == Instruction::AddrSpaceCast) {
       V = cast<Operator>(V)->getOperand(0);
     } else if (auto *GA = dyn_cast<GlobalAlias>(V)) {
-      if (StripKind == PSK_ZeroIndices || GA->isInterposable())
+      if (StripKind == PSK_ZeroIndices || StripKind == PSK_InBounds ||
+          GA->isInterposable())
         return V;
       V = GA->getAliasee();
     } else {
       if (auto CS = ImmutableCallSite(V)) {
         if (const Value *RV = CS.getReturnedArgOperand()) {
           V = RV;
           continue;
         }
const Value *Value::stripPointerCasts() const {
   return stripPointerCastsAndOffsets<PSK_ZeroIndicesAndAliases>(this);
 }
 
 const Value *Value::stripPointerCastsNoFollowAliases() const {
   return stripPointerCastsAndOffsets<PSK_ZeroIndices>(this);
 }
 
 const Value *Value::stripInBoundsConstantOffsets() const {
-  return stripPointerCastsAndOffsets<PSK_InBoundsConstantIndices>(this);
+  return stripPointerCastsAndOffsets<PSK_InBoundsConstantIndicesAndAliases>(
+      this);
 }
 
 const Value *Value::stripPointerCastsAndInvariantGroups() const {
   return stripPointerCastsAndOffsets<PSK_ZeroIndicesAndAliasesAndInvariantGroups>(
       this);
 }
 
 const Value *
do {
     }
     assert(V->getType()->isPointerTy() && "Unexpected operand type!");
   } while (Visited.insert(V).second);
 
   return V;
 }
 
 const Value *Value::stripInBoundsOffsets() const {
+  return stripPointerCastsAndOffsets<PSK_InBoundsAndAliases>(this);
+}
+
+const Value *Value::stripInBoundsOffsetsNoFollowAliases() const {
   return stripPointerCastsAndOffsets<PSK_InBounds>(this);
 }
 
 uint64_t Value::getPointerDereferenceableBytes(const DataLayout &DL,
                                                bool &CanBeNull) const {
   assert(getType()->isPointerTy() && "must be pointer");
 
   uint64_t DerefBytes = 0;

lib/Transforms/IPO/FunctionImport.cpp

for (auto &S : VI.getSummaryList())
       S->setLive(true);
     ++LiveSymbols;
     Worklist.push_back(VI);
   };
 
   while (!Worklist.empty()) {
     auto VI = Worklist.pop_back_val();
     for (auto &Summary : VI.getSummaryList()) {
-      GlobalValueSummary *Base = Summary->getBaseObject();
-      // Set base value live in case it is an alias.
+      auto *Base = Summary.get();
       Base->setLive(true);
+      while (auto *AS = dyn_cast<AliasSummary>(Base)) {
+        Base = &AS->getAliasee();
+        // Set aliasees live
+        Base->setLive(true);
+      }
       for (auto Ref : Base->refs())
         visit(Ref);
       if (auto *FS = dyn_cast<FunctionSummary>(Base))
         for (auto Call : FS->calls())
           visit(Call.first);
     }
   }
   Index.setWithGlobalValueDeadStripping();

test/ThinLTO/X86/alias_to_alias.ll

+; Test to ensure that alias to another alias is handled properly by
+; ThinLTO, specifically in the summary and the dead stripping handling.
+; REQUIRES: x86-registered-target
+
+; RUN: opt -module-summary %s -o %t.o
+
+; Make sure the alias chain is properly represented in summary.
+; RUN: llvm-dis -o - %t.o | FileCheck %s --check-prefix=DIS
+; DIS-DAG: [[FUNC:\^[1-3]]] = gv: (name: "aliasee", summaries: (function:
+; DIS-DAG: [[ALIAS1:\^[1-3]]] = gv: (name: "anotheralias", summaries: (alias: {{.*}}, aliasee: [[FUNC]]))
+; DIS-DAG: [[ALIAS2:\^[1-3]]] = gv: (name: "analias", summaries: (alias: {{.*}}, aliasee: [[ALIAS1]]))
+
+; RUN: llvm-lto2 run -o %t2 %t.o -r %t.o,aliasee,lx -r %t.o,analias,plx -r %t.o,anotheralias,pl -save-temps
+
+; Make sure none are dead stripped.
+; RUN: llvm-dis -o - %t2.1.2.internalize.bc | FileCheck %s
+; CHECK-DAG: @analias = dso_local alias void (), void ()* @anotheralias
+; CHECK-DAG: @anotheralias = internal alias void (), void ()* @aliasee
+; CHECK-DAG: define internal void @aliasee()
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@analias = alias void (), void ()* @anotheralias
+@anotheralias = alias void (), bitcast (void ()* @aliasee to void ()*)
+
+; Function Attrs: nounwind uwtable
+define void @aliasee() #0 {
+entry:
+    ret void
+}