[ThinLTO] Always import constants

[evgeny777]

Summary

Since D69561 we started to import readonly vars with non-trivial initializers. However this isn't always the case with compile time constants, because thin LTO analysis pass is not always detecting them as readonly. Such thing happens in two cases:

• constant object is referenced in initializer of some other object
• constant object is referenced by instruction which is not non-volatile load.

This patch forces read-only attribute for constants, regardless of their usage

[evgeny777]

Found a bug, will resubmit after fix and more thorough tests.

[evgeny777]

I reimplemented the patch, now using extra 'Constant' attribute in GlobalVarSummary.

The goal is to allow importing of constants, which are not being detected as 'ReadOnly'. Such constants are imported with available_externally linkage, still allowing compiler to make some optimizations along the way.

[evgeny777]

Patch was tested on Firefox and LLVM test suites. Both pass flawlessly

[steven_wu]

In D70404#1755124, @evgeny777 wrote:

I reimplemented the patch, now using extra 'Constant' attribute in GlobalVarSummary.

The goal is to allow importing of constants, which are not being detected as 'ReadOnly'. Such constants are imported with available_externally linkage, still allowing compiler to make some optimizations along the way.

Looks good at first glimpse. Out of curiosity, what problem do you run into when you unify ReadOnly with Constant?

[evgeny777]

Out of curiosity, what problem do you run into when you unify ReadOnly with Constant?

When constant is always treated as ReadOnly it is also always internalized. Now if constant 'A' references constant 'B' in initializer you also have to import constant 'B' when constant 'A' is imported. Otherwise there is linker undef,
because both A and B are internal. This requires recursive variable import which we currently lack.

The more serious problem however is that if constant is not unnamed_addr you can't internalize it at all, because you may end up with different copies of it, each having unique address. So in any case some extra attribute in
GlobalVarSummary is needed to indicate if we can internalize ReadOnly var or not.

[tejohnson]

Please update the patch description (still talks about forcing read only).

Presumably new flag should be added to llvm::computeLTOCacheKey where it already hashes the read/write only flags.

In D70404#1756261, @evgeny777 wrote:

Out of curiosity, what problem do you run into when you unify ReadOnly with Constant?

When constant is always treated as ReadOnly it is also always internalized. Now if constant 'A' references constant 'B' in initializer you also have to import constant 'B' when constant 'A' is imported. Otherwise there is linker undef,
because both A and B are internal. This requires recursive variable import which we currently lack.

The more serious problem however is that if constant is not unnamed_addr you can't internalize it at all, because you may end up with different copies of it, each having unique address. So in any case some extra attribute in
GlobalVarSummary is needed to indicate if we can internalize ReadOnly var or not.

I see a test for the case where the constant must be imported as promoted, but perhaps there should be one to ensure we can import as internal when possible?

llvm/lib/Analysis/ModuleSummaryAnalysis.cpp

636

Does the change to the write only initialization affect anything user visible? I was trying to reason through it but haven't convinced myself either way. I.e. will it affect which variables are internalized/zeroinitialized?

751

Should this be an assert rather than an if? Looks like we assume it is a GVar since we create a summary for such below. Not sure if def in inline asm can be an alias, but we certainly seem to assume not here.

llvm/test/ThinLTO/X86/import-constant.ll

2

Looks like it is promoted, not internalized?

16

outer is internalized and given zeroinitializer because it is write only I assume - can you add a comment to that effect here?

17

Check that also promoted in exporting module?

llvm/test/ThinLTO/X86/referenced_by_constant.ll

7

Looks like this comment should be updated now?

[evgeny777]

• Added test case for 'readonly' constants (import-ro-constant.ll)
• Added support for 'Constant' attribute to dot dumper
• Addressed other comments

llvm/lib/Analysis/ModuleSummaryAnalysis.cpp

636

I thought it made sense, because no variable can be both constant and writeonly. However setting it to false doesn't really change anything so I removed this.

751

Yep. It can be just cast<GlobalVariable> instead

[evgeny777]

Ping

[tejohnson]

Sorry for the delay, I lost track of this one.

llvm/lib/Analysis/ModuleSummaryAnalysis.cpp

636

Ok, true. I'm not sure why I was concerned - I'm ok with the earlier version of the code, agree that it doesn't really make sense to start out with marking it possibly WriteOnly if it cannot be.

llvm/test/ThinLTO/X86/import-constant.ll

2

Looking at this test again, I guess @_ZL3Obj must be getting internalized after promotion, which is why it gets optimized away in the OPT version. Could you check it directly after the internalization phase? I also had a couple other suggestions below.

[evgeny777]

llvm/test/ThinLTO/X86/import-constant.ll

2

Looking at this test again, I guess @_ZL3Obj must be getting internalized after promotion, which is why it gets optimized away in the OPT version.

It's been promoted, not internalized. However @_ZL3Obj is constant so it can be folded even when it has available_externally linkage

[evgeny777]

Addressed review comments

[tejohnson]

Can you re-upload the latest with context? It isn't available and therefore I can't compare the last two diffs.

[evgeny777]

Ah, sorry, uploaded wrong file.

[tejohnson]

LGTM

llvm/include/llvm/IR/ModuleSummaryIndex.h

void setVTableFuncs(VTableFuncList Funcs) {
     VTableFuncs = std::make_unique<VTableFuncList>(std::move(Funcs));
   }
 
   ArrayRef<VirtFuncOffset> vTableFuncs() const {
     if (VTableFuncs)
       return *VTableFuncs;
     return {};
   }
+
+  size_t numConstantRefs() const;
 };
 
 struct TypeTestResolution {
   /// Specifies which kind of type check we should emit for this byte array.
   /// See http://clang.llvm.org/docs/ControlFlowIntegrityDesign.html for full
   /// details on each kind of check; the enumerators are described with
   /// reference to that document.
   enum Kind {

llvm/lib/Analysis/ModuleSummaryAnalysis.cpp

 
 static bool isNonVolatileStore(const Instruction *I) {
   if (const auto *SI = dyn_cast<StoreInst>(I))
     return !SI->isVolatile();
 
   return false;
 }
 
+static void markRefsToConstantsAsReadOnly(std::vector<ValueInfo> &Refs,
+                                          size_t Count) {
+  auto IsConstantValue = [](const GlobalValue *GV) {
+    if (const auto *GVar = dyn_cast<GlobalVariable>(GV))
+      return GVar->isConstant();
+
+    if (const auto *GA = dyn_cast<GlobalAlias>(GV))
+      if (const auto *GVar = dyn_cast<GlobalVariable>(GA->getAliasee()))
+        return GVar->isConstant();
+    return false;
+  };
+
+  size_t NumConstRefs = 0;
+  // Detect readonly references and group them at the end of
+  // Refs array.
+  for (size_t N = 0; N < Count && !Refs[N].isReadOnly();) {
+    if (!IsConstantValue(Refs[N].getValue())) {
+      ++N;
+      continue;
+    }
+
+    Refs[N].setReadOnly();
+    std::swap(Refs[N], Refs[Count - NumConstRefs - 1]);
+    ++NumConstRefs;
+  }
+}
+
 static void computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M,
                                    const Function &F, BlockFrequencyInfo *BFI,
                                    ProfileSummaryInfo *PSI, DominatorTree &DT,
                                    bool HasLocalsInUsedOrAsm,
                                    DenseSet<GlobalValue::GUID> &CantBePromoted,
                                    bool IsThinLTO) {
   // Summary not currently supported for anonymous functions, they should
   // have been named.
if (IsThinLTO) {
 
     // If both load and store instruction reference the same variable
     // we won't be able to optimize it. Add all such reference edges
     // to RefEdges set.
     for (auto &VI : StoreRefEdges)
       if (LoadRefEdges.remove(VI))
         RefEdges.insert(VI);
 
-    unsigned RefCnt = RefEdges.size();
+    unsigned RefCnt = RefEdges.size(), UntypedRefCnt = RefCnt;
     // All new reference edges inserted in two loops below are either
     // read or write only. They will be grouped in the end of RefEdges
     // vector, so we can use a single integer value to identify them.
     for (auto &VI : LoadRefEdges)
       RefEdges.insert(VI);
 
     unsigned FirstWORef = RefEdges.size();
     for (auto &VI : StoreRefEdges)
       RefEdges.insert(VI);
 
     Refs = RefEdges.takeVector();
     for (; RefCnt < FirstWORef; ++RefCnt)
       Refs[RefCnt].setReadOnly();
 
     for (; RefCnt < Refs.size(); ++RefCnt)
       Refs[RefCnt].setWriteOnly();
+
+    // Mark all references to constant objects as readonly.
+    markRefsToConstantsAsReadOnly(Refs, UntypedRefCnt);
   } else {
     Refs = RefEdges.takeVector();
   }
   // Explicit add hot edges to enforce importing for designated GUIDs for
   // sample PGO, to enable the same inlines as the profiled optimized binary.
   for (auto &I : F.getImportGUIDs())
     CallGraphEdges[Index.getOrInsertValueInfo(I)].updateHotness(
         ForceSummaryEdgesCold == FunctionSummary::FSHT_All
if (!Types.empty()) {
       recordTypeIdCompatibleVtableReferences(Index, V, Types);
     }
   }
 
   // Don't mark variables we won't be able to internalize as read/write-only.
   bool CanBeInternalized =
       !V.hasComdat() && !V.hasAppendingLinkage() && !V.isInterposable() &&
       !V.hasAvailableExternallyLinkage() && !V.hasDLLExportStorageClass();
+  auto Refs = RefEdges.takeVector();
+  markRefsToConstantsAsReadOnly(Refs, Refs.size());
   GlobalVarSummary::GVarFlags VarFlags(CanBeInternalized, CanBeInternalized);
-  auto GVarSummary = std::make_unique<GlobalVarSummary>(Flags, VarFlags,
-                                                         RefEdges.takeVector());
+  auto GVarSummary = std::make_unique<GlobalVarSummary>(Flags, VarFlags, Refs);
   if (NonRenamableLocal)
     CantBePromoted.insert(V.getGUID());
   if (HasBlockAddress)
     GVarSummary->setNotEligibleToImport();
   if (!VTableFuncs.empty())
     GVarSummary->setVTableFuncs(VTableFuncs);
   Index.addGlobalValueSummary(V, std::move(GVarSummary));
 }
 
ModuleSymbolTable::CollectAsmSymbols(
                     ArrayRef<GlobalValue::GUID>{},
                     ArrayRef<FunctionSummary::VFuncId>{},
                     ArrayRef<FunctionSummary::VFuncId>{},
                     ArrayRef<FunctionSummary::ConstVCall>{},
                     ArrayRef<FunctionSummary::ConstVCall>{});
             Index.addGlobalValueSummary(*GV, std::move(Summary));
           } else {
             std::unique_ptr<GlobalVarSummary> Summary =
                 std::make_unique<GlobalVarSummary>(
                     GVFlags, GlobalVarSummary::GVarFlags(false, false),
                     ArrayRef<ValueInfo>{});
             Index.addGlobalValueSummary(*GV, std::move(Summary));
           }
         });
   }
 
   bool IsThinLTO = true;

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

// Parse version
     Expected<unsigned> MaybeRecord = Stream.readRecord(Entry.ID, Record);
     if (!MaybeRecord)
       return MaybeRecord.takeError();
     if (MaybeRecord.get() != bitc::FS_VERSION)
       return error("Invalid Summary Block: version expected");
   }
   const uint64_t Version = Record[0];
   const bool IsOldProfileFormat = Version == 1;
-  if (Version < 1 || Version > 8)
+  if (Version < 1 || Version > 9)
     return error("Invalid summary version " + Twine(Version) +
-                 ". Version should be in the range [1-7].");
+                 ". Version should be in the range [1-9].");
   Record.clear();
 
   // Keep around the last seen summary to be used when we see an optional
   // "OriginalName" attachement.
   GlobalValueSummary *LastSeenSummary = nullptr;
   GlobalValue::GUID LastSeenGUID = 0;
 
   // We can expect to see any number of type ID information records before
while (true) {
     // FS_PERMODULE_GLOBALVAR_INIT_REFS: [valueid, flags, varflags, n x valueid]
     case bitc::FS_PERMODULE_GLOBALVAR_INIT_REFS: {
       unsigned ValueID = Record[0];
       uint64_t RawFlags = Record[1];
       unsigned RefArrayStart = 2;
       GlobalVarSummary::GVarFlags GVF(/* ReadOnly */ false,
                                       /* WriteOnly */ false);
       auto Flags = getDecodedGVSummaryFlags(RawFlags, Version);
+      size_t NumConstRefs = 0;
       if (Version >= 5) {
         GVF = getDecodedGVarFlags(Record[2]);
         RefArrayStart = 3;
+        if (Version >= 9) {
+          NumConstRefs = Record[3];
+          RefArrayStart = 4;
+        }
       }
       std::vector<ValueInfo> Refs =
           makeRefList(ArrayRef<uint64_t>(Record).slice(RefArrayStart));
+      setSpecialRefs(Refs, NumConstRefs, 0);
       auto FS =
           std::make_unique<GlobalVarSummary>(Flags, GVF, std::move(Refs));
       FS->setModulePath(getThisModule()->first());
       auto GUID = getValueInfoFromValueId(ValueID);
       FS->setOriginalName(GUID.second);
       TheIndex.addGlobalValueSummary(GUID.first, std::move(FS));
       break;
     }
     // FS_PERMODULE_VTABLE_GLOBALVAR_INIT_REFS: [valueid, flags, varflags,
     //                        numrefs, numrefs x valueid,
     //                        n x (valueid, offset)]
     case bitc::FS_PERMODULE_VTABLE_GLOBALVAR_INIT_REFS: {
       unsigned ValueID = Record[0];
       uint64_t RawFlags = Record[1];
       GlobalVarSummary::GVarFlags GVF = getDecodedGVarFlags(Record[2]);
-      unsigned NumRefs = Record[3];
+      unsigned NumRefs = Record[3], NumConstRefs = 0;
       unsigned RefListStartIndex = 4;
+      if (Version >= 9) {
+        NumConstRefs = Record[4];
+        RefListStartIndex = 5;
+      }
       unsigned VTableListStartIndex = RefListStartIndex + NumRefs;
       auto Flags = getDecodedGVSummaryFlags(RawFlags, Version);
       std::vector<ValueInfo> Refs = makeRefList(
           ArrayRef<uint64_t>(Record).slice(RefListStartIndex, NumRefs));
+      setSpecialRefs(Refs, NumConstRefs, 0);
       VTableFuncList VTableFuncs;
       for (unsigned I = VTableListStartIndex, E = Record.size(); I != E; ++I) {
         ValueInfo Callee = getValueInfoFromValueId(Record[I]).first;
         uint64_t Offset = Record[++I];
         VTableFuncs.push_back({Callee, Offset});
       }
       auto VS =
           std::make_unique<GlobalVarSummary>(Flags, GVF, std::move(Refs));
while (true) {
     case bitc::FS_COMBINED_GLOBALVAR_INIT_REFS: {
       unsigned ValueID = Record[0];
       uint64_t ModuleId = Record[1];
       uint64_t RawFlags = Record[2];
       unsigned RefArrayStart = 3;
       GlobalVarSummary::GVarFlags GVF(/* ReadOnly */ false,
                                       /* WriteOnly */ false);
       auto Flags = getDecodedGVSummaryFlags(RawFlags, Version);
+      size_t NumConstRefs = 0;
       if (Version >= 5) {
         GVF = getDecodedGVarFlags(Record[3]);
         RefArrayStart = 4;
+        if (Version >= 9) {
+          NumConstRefs = Record[4];
+          RefArrayStart = 5;
+        }
       }
       std::vector<ValueInfo> Refs =
           makeRefList(ArrayRef<uint64_t>(Record).slice(RefArrayStart));
+      setSpecialRefs(Refs, NumConstRefs, 0);
       auto FS =
           std::make_unique<GlobalVarSummary>(Flags, GVF, std::move(Refs));
       LastSeenSummary = FS.get();
       FS->setModulePath(ModuleIdMap[ModuleId]);
       ValueInfo VI = getValueInfoFromValueId(ValueID).first;
       LastSeenGUID = VI.getGUID();
       TheIndex.addGlobalValueSummary(VI, std::move(FS));
       break;

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

void ModuleBitcodeWriterBase::writeModuleLevelReferences(
   GlobalVarSummary *VS = cast<GlobalVarSummary>(Summary);
   NameVals.push_back(getEncodedGVSummaryFlags(VS->flags()));
   NameVals.push_back(getEncodedGVarFlags(VS->varflags()));
 
   auto VTableFuncs = VS->vTableFuncs();
   if (!VTableFuncs.empty())
     NameVals.push_back(VS->refs().size());
 
-  unsigned SizeBeforeRefs = NameVals.size();
+  NameVals.push_back(VS->numConstantRefs());
   for (auto &RI : VS->refs())
     NameVals.push_back(VE.getValueID(RI.getValue()));
-  // Sort the refs for determinism output, the vector returned by FS->refs() has
-  // been initialized from a DenseSet.
-  llvm::sort(NameVals.begin() + SizeBeforeRefs, NameVals.end());
 
   if (VTableFuncs.empty())
     Stream.EmitRecord(bitc::FS_PERMODULE_GLOBALVAR_INIT_REFS, NameVals,
                       FSModRefsAbbrev);
   else {
     // VTableFuncs pairs should already be sorted by offset.
     for (auto &P : VTableFuncs) {
       NameVals.push_back(VE.getValueID(P.FuncVI.getValue()));
       NameVals.push_back(P.VTableOffset);
     }
 
     Stream.EmitRecord(bitc::FS_PERMODULE_VTABLE_GLOBALVAR_INIT_REFS, NameVals,
                       FSModVTableRefsAbbrev);
   }
   NameVals.clear();
 }
 
 // Current version for the summary.
 // This is bumped whenever we introduce changes in the way some record are
 // interpreted, like flags for instance.
-static const uint64_t INDEX_VERSION = 8;
+static const uint64_t INDEX_VERSION = 9;
 
 /// Emit the per-module summary section alongside the rest of
 /// the module's bitcode.
 void ModuleBitcodeWriterBase::writePerModuleGlobalValueSummary() {
   // By default we compile with ThinLTO if the module has a summary, but the
   // client can request full LTO with a module flag.
   bool IsThinLTO = true;
   if (auto *MD =
void ModuleBitcodeWriterBase::writePerModuleGlobalValueSummary() {
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));
   unsigned FSCallsAbbrev = Stream.EmitAbbrev(std::move(Abbv));
 
   // Abbrev for FS_PERMODULE_GLOBALVAR_INIT_REFS.
   Abbv = std::make_shared<BitCodeAbbrev>();
   Abbv->Add(BitCodeAbbrevOp(bitc::FS_PERMODULE_GLOBALVAR_INIT_REFS));
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8)); // valueid
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // flags
+  Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 2)); // varFlags
+  Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 4)); // numConstantRefs
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));  // valueids
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));
   unsigned FSModRefsAbbrev = Stream.EmitAbbrev(std::move(Abbv));
 
   // Abbrev for FS_PERMODULE_VTABLE_GLOBALVAR_INIT_REFS.
   Abbv = std::make_shared<BitCodeAbbrev>();
   Abbv->Add(BitCodeAbbrevOp(bitc::FS_PERMODULE_VTABLE_GLOBALVAR_INIT_REFS));
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8)); // valueid
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // flags
+  Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 2)); // varFlags
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 4)); // numrefs
+  Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 4)); // numConstantRefs
   // numrefs x valueid, n x (valueid , offset)
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));
   unsigned FSModVTableRefsAbbrev = Stream.EmitAbbrev(std::move(Abbv));
 
   // Abbrev for FS_ALIAS.
   Abbv = std::make_shared<BitCodeAbbrev>();
   Abbv->Add(BitCodeAbbrevOp(bitc::FS_ALIAS));
void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
   unsigned FSCallsProfileAbbrev = Stream.EmitAbbrev(std::move(Abbv));
 
   // Abbrev for FS_COMBINED_GLOBALVAR_INIT_REFS.
   Abbv = std::make_shared<BitCodeAbbrev>();
   Abbv->Add(BitCodeAbbrevOp(bitc::FS_COMBINED_GLOBALVAR_INIT_REFS));
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));   // valueid
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));   // modid
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6));   // flags
+  Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 2));   // varFlags
+  Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 4));   // numConstantRefs
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));    // valueids
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));
   unsigned FSModRefsAbbrev = Stream.EmitAbbrev(std::move(Abbv));
 
   // Abbrev for FS_COMBINED_ALIAS.
   Abbv = std::make_shared<BitCodeAbbrev>();
   Abbv->Add(BitCodeAbbrevOp(bitc::FS_COMBINED_ALIAS));
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));   // valueid
if (auto *AS = dyn_cast<AliasSummary>(S)) {
       return;
     }
 
     if (auto *VS = dyn_cast<GlobalVarSummary>(S)) {
       NameVals.push_back(*ValueId);
       NameVals.push_back(Index.getModuleId(VS->modulePath()));
       NameVals.push_back(getEncodedGVSummaryFlags(VS->flags()));
       NameVals.push_back(getEncodedGVarFlags(VS->varflags()));
+      NameVals.push_back(VS->numConstantRefs());
       for (auto &RI : VS->refs()) {
         auto RefValueId = getValueId(RI.getGUID());
         if (!RefValueId)
           continue;
         NameVals.push_back(*RefValueId);
       }
 
       // Emit the finished record.

llvm/lib/IR/ModuleSummaryIndex.cpp

bool ValueInfo::canAutoHide() const {
   // Can only auto hide if all copies are eligible to auto hide.
   return getSummaryList().size() &&
          llvm::all_of(getSummaryList(),
                       [](const std::unique_ptr<GlobalValueSummary> &Summary) {
                         return Summary->canAutoHide();
                       });
 }
 
+size_t GlobalVarSummary::numConstantRefs() const {
+  // Constant references are grouped in the end of refs() array.
+  int N;
+  auto Refs = refs();
+  for (N = Refs.size() - 1; N >= 0 && Refs[N].isReadOnly(); --N) {
+  }
+  return Refs.size() - N - 1;
+}
+
 // Gets the number of readonly and writeonly refs in RefEdgeList
 std::pair<unsigned, unsigned> FunctionSummary::specialRefCounts() const {
   // Here we take advantage of having all readonly and writeonly references
   // located in the end of the RefEdgeList.
   auto Refs = refs();
   unsigned RORefCnt = 0, WORefCnt = 0;
   int I;
   for (I = Refs.size() - 1; I >= 0 && Refs[I].isWriteOnly(); --I)
for (auto &I : SummaryList)
     if (isGlobalValueLive(I.get()))
       return true;
   return false;
 }
 
 static void propagateAttributesToRefs(GlobalValueSummary *S) {
   // If reference is not readonly or writeonly then referenced summary is not
   // read/writeonly either. Note that:
-  // - All references from GlobalVarSummary are conservatively considered as
-  //   not readonly or writeonly. Tracking them properly requires more complex
-  //   analysis then we have now.
+  // - References from GlobalVarSummary can be readonly if they point to
+  //   constant objects. Those can't be writeonly.
   //
   // - AliasSummary objects have no refs at all so this function is a no-op
   //   for them.
   for (auto &VI : S->refs()) {
-    assert(VI.getAccessSpecifier() == 0 || isa<FunctionSummary>(S));
+    assert(!VI.isWriteOnly() || isa<FunctionSummary>(S));
     for (auto &Ref : VI.getSummaryList())
       // If references to alias is not read/writeonly then aliasee
       // is not read/writeonly
       if (auto *GVS = dyn_cast<GlobalVarSummary>(Ref->getBaseObject())) {
         if (!VI.isReadOnly())
           GVS->setReadOnly(false);
         if (!VI.isWriteOnly())
           GVS->setWriteOnly(false);

llvm/test/Assembler/thinlto-summary.ll

 ^4 = gv: (guid: 3, summaries: (function: (module: ^0, flags: (linkage: internal, notEligibleToImport: 0, live: 0, dsoLocal: 1), insts: 1)))
 ; Make this one an alias with a forward reference to aliasee.
 ^5 = gv: (guid: 4, summaries: (alias: (module: ^0, flags: (linkage: private, notEligibleToImport: 0, live: 0, dsoLocal: 1), aliasee: ^14)))
 ^6 = gv: (guid: 5, summaries: (function: (module: ^0, flags: (linkage: available_externally, notEligibleToImport: 0, live: 0, dsoLocal: 0), insts: 1)))
 ^7 = gv: (guid: 6, summaries: (function: (module: ^0, flags: (linkage: linkonce, notEligibleToImport: 0, live: 0, dsoLocal: 0), insts: 1)))
 ^8 = gv: (guid: 7, summaries: (function: (module: ^0, flags: (linkage: linkonce_odr, notEligibleToImport: 0, live: 0, dsoLocal: 0), insts: 1)))
 ^9 = gv: (guid: 8, summaries: (function: (module: ^0, flags: (linkage: weak_odr, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 1), insts: 1)))
 ^10 = gv: (guid: 9, summaries: (function: (module: ^0, flags: (linkage: weak, notEligibleToImport: 0, live: 0, dsoLocal: 0), insts: 1)))
-^11 = gv: (guid: 10, summaries: (variable: (module: ^0, flags: (linkage: common, notEligibleToImport: 0, live: 0, dsoLocal: 0), varFlags: (readonly: 0))))
+^11 = gv: (guid: 10, summaries: (variable: (module: ^0, flags: (linkage: common, notEligibleToImport: 0, live: 0, dsoLocal: 0), varFlags: (readonly: 0), refs: (readonly ^13, ^14))))
 ; Test appending globel variable with reference (tests backward reference on
 ; refs).
 ^12 = gv: (guid: 11, summaries: (variable: (module: ^0, flags: (linkage: appending, notEligibleToImport: 0, live: 0, dsoLocal: 0), varFlags: (readonly: 0), refs: (^4))))
 
 ; Test a referenced global variable.
 ^13 = gv: (guid: 12, summaries: (variable: (module: ^0, flags: (linkage: external, notEligibleToImport: 0, live: 0, dsoLocal: 0), varFlags: (readonly: 1))))
 
 ; Test a dsoLocal variable.

 ; CHECK: ^3 = gv: (guid: 2, summaries: (function: (module: ^1, flags: (linkage: external, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 10, calls: ((callee: ^15)))))
 ; CHECK: ^4 = gv: (guid: 3, summaries: (function: (module: ^0, flags: (linkage: internal, notEligibleToImport: 0, live: 0, dsoLocal: 1, canAutoHide: 0), insts: 1)))
 ; CHECK: ^5 = gv: (guid: 4, summaries: (alias: (module: ^0, flags: (linkage: private, notEligibleToImport: 0, live: 0, dsoLocal: 1, canAutoHide: 0), aliasee: ^14)))
 ; CHECK: ^6 = gv: (guid: 5, summaries: (function: (module: ^0, flags: (linkage: available_externally, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 1)))
 ; CHECK: ^7 = gv: (guid: 6, summaries: (function: (module: ^0, flags: (linkage: linkonce, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 1)))
 ; CHECK: ^8 = gv: (guid: 7, summaries: (function: (module: ^0, flags: (linkage: linkonce_odr, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 1)))
 ; CHECK: ^9 = gv: (guid: 8, summaries: (function: (module: ^0, flags: (linkage: weak_odr, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 1), insts: 1)))
 ; CHECK: ^10 = gv: (guid: 9, summaries: (function: (module: ^0, flags: (linkage: weak, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 1)))
-; CHECK: ^11 = gv: (guid: 10, summaries: (variable: (module: ^0, flags: (linkage: common, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), varFlags: (readonly: 0, writeonly: 0))))
+; CHECK: ^11 = gv: (guid: 10, summaries: (variable: (module: ^0, flags: (linkage: common, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), varFlags: (readonly: 0, writeonly: 0), refs: (^14, readonly ^13))))
 ; CHECK: ^12 = gv: (guid: 11, summaries: (variable: (module: ^0, flags: (linkage: appending, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), varFlags: (readonly: 0, writeonly: 0), refs: (^4))))
 ; CHECK: ^13 = gv: (guid: 12, summaries: (variable: (module: ^0, flags: (linkage: external, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), varFlags: (readonly: 1, writeonly: 0))))
 ; CHECK: ^14 = gv: (guid: 13, summaries: (variable: (module: ^0, flags: (linkage: external, notEligibleToImport: 0, live: 0, dsoLocal: 1, canAutoHide: 0), varFlags: (readonly: 0, writeonly: 0))))
 ; CHECK: ^15 = gv: (guid: 14, summaries: (function: (module: ^1, flags: (linkage: external, notEligibleToImport: 1, live: 1, dsoLocal: 0, canAutoHide: 0), insts: 1, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 1, alwaysInline: 0))))
 ; CHECK: ^16 = gv: (guid: 15, summaries: (function: (module: ^1, flags: (linkage: external, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 1, funcFlags: (readNone: 1, readOnly: 0, noRecurse: 1, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 1))))
 ; CHECK: ^17 = gv: (guid: 16, summaries: (function: (module: ^1, flags: (linkage: external, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 1, funcFlags: (readNone: 0, readOnly: 1, noRecurse: 0, returnDoesNotAlias: 1, noInline: 0, alwaysInline: 0), calls: ((callee: ^15)))))
 ; CHECK: ^18 = gv: (guid: 17, summaries: (alias: (module: ^0, flags: (linkage: external, notEligibleToImport: 0, live: 0, dsoLocal: 1, canAutoHide: 0), aliasee: ^14)))
 ; CHECK: ^19 = gv: (guid: 18, summaries: (function: (module: ^0, flags: (linkage: external, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 4, typeIdInfo: (typeTests: (^24, ^26)))))

llvm/test/Bitcode/summary_version.ll

 ; Check summary versioning
 ; RUN: opt  -module-summary  %s -o - | llvm-bcanalyzer -dump | FileCheck %s
 
 ; CHECK: <GLOBALVAL_SUMMARY_BLOCK
-; CHECK: <VERSION op0=8/>
+; CHECK: <VERSION op0=9/>
 
 
 
 ; Need a function for the summary to be populated.
 define void @foo() {
     ret void
 }

llvm/test/Bitcode/thinlto-function-summary-refgraph.ll

 ; CHECK-DAG:    <PERMODULE {{.*}} op0=8 op1=72 {{.*}} op4=0 op5=0 op6=0 op7=3/>
 ; Function Z contains call to func2, and ensures we don't incorrectly add
 ; a reference to it when reached while analyzing subsequent use of its return
 ; value:
 ; op0=Z op4=func2
 ; CHECK-DAG:    <PERMODULE {{.*}} op0=9 op1=3 {{.*}} op4=0 op5=0 op6=0 op7=3/>
 ; Variable bar initialization contains address reference to func:
 ; op0=bar op2=func
-; CHECK-DAG:    <PERMODULE_GLOBALVAR_INIT_REFS {{.*}} op0=0 op1=0 op2=3 op3=2/>
+; CHECK-DAG:    <PERMODULE_GLOBALVAR_INIT_REFS {{.*}} op0=0 op1=0 op2=3 op3=0 op4=2/>
 ; CHECK:  </GLOBALVAL_SUMMARY_BLOCK>
 
 ; CHECK: <STRTAB_BLOCK
 ; CHECK-NEXT: blob data = 'barglobalvarfuncfunc2foofunc3WXYZllvm.ctpop.i8main{{.*}}'
 
 ; ModuleID = 'thinlto-function-summary-refgraph.ll'
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"

llvm/test/ThinLTO/X86/Inputs/import-constant.ll

+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+%struct.S = type { i32, i32, i32* }
+%struct.Q = type { %struct.S* }
+
+@val = dso_local global i32 42, align 4
+@_ZL3Obj = internal constant %struct.S { i32 4, i32 8, i32* @val }, align 8
+@outer = dso_local local_unnamed_addr global %struct.Q { %struct.S* @_ZL3Obj }, align 8
+
+define dso_local nonnull %struct.S* @_Z6getObjv() local_unnamed_addr {
+entry:
+  store %struct.S* null, %struct.S** getelementptr inbounds (%struct.Q, %struct.Q* @outer, i64 0, i32 0), align 8
+  ret %struct.S* @_ZL3Obj
+}

llvm/test/ThinLTO/X86/devirt.ll

 ; Check that we don't have module flag when splitting not enabled for ThinLTO,
 ; and that we generate summary information needed for index-based WPD.
 ; RUN: llvm-dis -o - %t2.o | FileCheck %s --check-prefix=NOENABLESPLITFLAG
 ; NOENABLESPLITFLAG-DAG: !{i32 1, !"EnableSplitLTOUnit", i32 0}
 ; NOENABLESPLITFLAG-DAG: [[An:\^[0-9]+]] = gv: (name: "_ZN1A1nEi"
 ; NOENABLESPLITFLAG-DAG: [[Bf:\^[0-9]+]] = gv: (name: "_ZN1B1fEi"
 ; NOENABLESPLITFLAG-DAG: [[Cf:\^[0-9]+]] = gv: (name: "_ZN1C1fEi"
 ; NOENABLESPLITFLAG-DAG: [[Dm:\^[0-9]+]] = gv: (name: "_ZN1D1mEi"
-; NOENABLESPLITFLAG-DAG: [[B:\^[0-9]+]] = gv: (name: "_ZTV1B", {{.*}} vTableFuncs: ((virtFunc: [[Bf]], offset: 16), (virtFunc: [[An]], offset: 24)), refs: ([[Bf]], [[An]])
+; NOENABLESPLITFLAG-DAG: [[B:\^[0-9]+]] = gv: (name: "_ZTV1B", {{.*}} vTableFuncs: ((virtFunc: [[Bf]], offset: 16), (virtFunc: [[An]], offset: 24)), refs: ([[An]], [[Bf]])
 ; NOENABLESPLITFLAG-DAG: [[C:\^[0-9]+]] = gv: (name: "_ZTV1C", {{.*}} vTableFuncs: ((virtFunc: [[Cf]], offset: 16), (virtFunc: [[An]], offset: 24)), refs: ([[An]], [[Cf]])
 ; NOENABLESPLITFLAG-DAG: [[D:\^[0-9]+]] = gv: (name: "_ZTV1D", {{.*}} vTableFuncs: ((virtFunc: [[Dm]], offset: 16)), refs: ([[Dm]])
 ; NOENABLESPLITFLAG-DAG: typeidCompatibleVTable: (name: "_ZTS1A", summary: ((offset: 16, [[B]]), (offset: 16, [[C]])))
 ; NOENABLESPLITFLAG-DAG: typeidCompatibleVTable: (name: "_ZTS1B", summary: ((offset: 16, [[B]])))
 ; NOENABLESPLITFLAG-DAG: typeidCompatibleVTable: (name: "_ZTS1C", summary: ((offset: 16, [[C]])))
 ; Type Id on _ZTV1D should have been promoted
 ; NOENABLESPLITFLAG-DAG: typeidCompatibleVTable: (name: "1${{.*}}", summary: ((offset: 16, [[D]])))
 

llvm/test/ThinLTO/X86/devirt2.ll

 ; Check that we don't have module flag when splitting not enabled for ThinLTO,
 ; and that we generate summary information needed for index-based WPD.
 ; RUN: llvm-dis -o - %t4.o | FileCheck %s --check-prefix=NOENABLESPLITFLAG
 ; NOENABLESPLITFLAG-DAG: !{i32 1, !"EnableSplitLTOUnit", i32 0}
 ; NOENABLESPLITFLAG-DAG: [[An:\^[0-9]+]] = gv: (name: "_ZN1A1nEi"
 ; NOENABLESPLITFLAG-DAG: [[Bf:\^[0-9]+]] = gv: (name: "_ZN1B1fEi"
 ; NOENABLESPLITFLAG-DAG: [[Cf:\^[0-9]+]] = gv: (name: "_ZN1C1fEi"
 ; NOENABLESPLITFLAG-DAG: [[Dm:\^[0-9]+]] = gv: (name: "_ZN1D1mEi"
-; NOENABLESPLITFLAG-DAG: [[B:\^[0-9]+]] = gv: (name: "_ZTV1B", {{.*}} vTableFuncs: ((virtFunc: [[Bf]], offset: 16), (virtFunc: [[An]], offset: 24)), refs: ([[Bf]], [[An]])
+; NOENABLESPLITFLAG-DAG: [[B:\^[0-9]+]] = gv: (name: "_ZTV1B", {{.*}} vTableFuncs: ((virtFunc: [[Bf]], offset: 16), (virtFunc: [[An]], offset: 24)), refs: ([[An]], [[Bf]])
 ; NOENABLESPLITFLAG-DAG: [[C:\^[0-9]+]] = gv: (name: "_ZTV1C", {{.*}} vTableFuncs: ((virtFunc: [[Cf]], offset: 16), (virtFunc: [[An]], offset: 24)), refs: ([[An]], [[Cf]])
 ; NOENABLESPLITFLAG-DAG: [[D:\^[0-9]+]] = gv: (name: "_ZTV1D", {{.*}} vTableFuncs: ((virtFunc: [[Dm]], offset: 16)), refs: ([[Dm]])
 ; NOENABLESPLITFLAG-DAG: typeidCompatibleVTable: (name: "_ZTS1A", summary: ((offset: 16, [[B]]), (offset: 16, [[C]])))
 ; NOENABLESPLITFLAG-DAG: typeidCompatibleVTable: (name: "_ZTS1B", summary: ((offset: 16, [[B]])))
 ; NOENABLESPLITFLAG-DAG: typeidCompatibleVTable: (name: "_ZTS1C", summary: ((offset: 16, [[C]])))
 ; NOENABLESPLITFLAG-DAG: typeidCompatibleVTable: (name: "_ZTS1D", summary: ((offset: 16, [[D]])))
 
 ; Legacy PM, Index based WPD

llvm/test/ThinLTO/X86/import-constant.ll

+; Check that we internalize constant object in the destination module
+; even when it is referenced in some other GV initializer and/or is
+; used by store instructions.
+; RUN: opt -thinlto-bc %s -o %t1.bc
+; RUN: opt -thinlto-bc %p/Inputs/import-constant.ll -o %t2.bc
+; RUN: llvm-lto2 run -save-temps %t1.bc %t2.bc -o %t-out \
+; RUN:    -r=%t1.bc,main,plx \
+; RUN:    -r=%t1.bc,_Z6getObjv,l \
+; RUN:    -r=%t2.bc,_Z6getObjv,pl \
+; RUN:    -r=%t2.bc,val,pl \
+; RUN:    -r=%t2.bc,outer,pl
+; RUN: llvm-dis %t-out.1.3.import.bc -o - | FileCheck %s --check-prefix=IMPORT
+; RUN: llvm-dis %t-out.1.4.opt.bc -o - | FileCheck %s --check-prefix=OPT
+
+; IMPORT:      @outer = internal local_unnamed_addr global %struct.Q zeroinitializer
+; IMPORT-NEXT: @_ZL3Obj.llvm.{{.*}} = internal constant %struct.S { i32 4, i32 8, i32* @val }
+; IMPORT-NEXT: @val = external dso_local global i32
+
+; OPT: @outer = internal unnamed_addr global %struct.Q zeroinitializer
+
+; OPT:      define dso_local i32 @main()
+; OPT-NEXT: entry:
+; OPT-NEXT:   store %struct.S* null, %struct.S** getelementptr inbounds (%struct.Q, %struct.Q* @outer, i64 0, i32 0)
+; OPT-NEXT:   ret i32 12
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+%struct.S = type { i32, i32, i32* }
+
+define dso_local i32 @main() local_unnamed_addr {
+entry:
+  %call = tail call %struct.S* @_Z6getObjv()
+  %d = getelementptr inbounds %struct.S, %struct.S* %call, i64 0, i32 0
+  %0 = load i32, i32* %d, align 8
+  %v = getelementptr inbounds %struct.S, %struct.S* %call, i64 0, i32 1
+  %1 = load i32, i32* %v, align 4
+  %add = add nsw i32 %1, %0
+  ret i32 %add
+}
+
+declare dso_local %struct.S* @_Z6getObjv() local_unnamed_addr

llvm/test/ThinLTO/X86/referenced_by_constant.ll

 ; Do setup work for all below tests: generate bitcode and combined index
 ; RUN: opt -module-summary %s -o %t.bc
 ; RUN: opt -module-summary %p/Inputs/referenced_by_constant.ll -o %t2.bc
 ; RUN: llvm-lto -thinlto-action=thinlink -o %t3.bc %t.bc %t2.bc
 
 ; Check the import side: we currently only import bar() (with a future
 ; enhancement to identify constants in the summary, we should mark
 ; @someglobal/@someglobal2 for import as a local copy, which would
 ; cause @referencedbyglobal and @localreferencedbyglobal to be exported
 ; and promoted).
 ; RUN: llvm-lto -thinlto-action=import %t.bc -thinlto-index=%t3.bc -o - | llvm-dis -o -   | FileCheck %s --check-prefix=IMPORT
-; IMPORT: @someglobal.llvm.0 = external hidden unnamed_addr constant
-; IMPORT: @someglobal2.llvm.0 = external hidden unnamed_addr constant
+; IMPORT: @someglobal.llvm.0 = internal unnamed_addr constant i8* bitcast (void ()* @referencedbyglobal to i8*) #0
+; IMPORT: @someglobal2.llvm.0 = internal unnamed_addr constant i8* bitcast (void ()* @localreferencedbyglobal.llvm.0 to i8*) #0
 ; IMPORT: define available_externally void @bar()
+; IMPORT: attributes #0 = { "thinlto-internalize" } 
 
 ; Check the export side: we currently only export bar(), which causes
 ; @someglobal and @someglobal2 to be promoted (see above).
 ; RUN: llvm-lto -thinlto-action=promote %t2.bc -thinlto-index=%t3.bc -o - | llvm-dis -o -   | FileCheck %s --check-prefix=EXPORT
 ; EXPORT: @someglobal.llvm.0 = hidden unnamed_addr constant
 ; EXPORT: @someglobal2.llvm.0 = hidden unnamed_addr constant
 ; EXPORT: define void @referencedbyglobal()
-; EXPORT: define internal void @localreferencedbyglobal()
+; EXPORT: define hidden void @localreferencedbyglobal
 
 target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.11.0"
 
 declare void @bar()
 
 define void @foo() {
     call void @bar()
     ret void
 }