Add GUID-ValueID map and original name to ThinLTO summary.

[haojiewang]

Summary

For now ThinLTO summary file has the same size of IR bitcode file, while it should be much smaller than that, since only the ModuleSummaryIndex, IRSymtab and Strtab should be written in it. GUID-ValueID map and original name are required in ThinLink phase, which are not included in module summary. If we want to reduce the summary file size, we should add these records into module summary, so in the next patch, we can do the reduction based on this change.

[haojiewang]

We need the GUID-ValueID map in ThinLink phase because in summary we only record the ValueID to identify every record. To get the mapping between this ValueID and GlobalValue's GUID, we need this GUID-ValueID map. In the revision before, we get this map by Value Symbol Table(VST), but there is some redundancy information in this block. In the future revision, only SummaryIndex, IRSymtab and Strtab will be recorded in summary file to reduce the size of it, so we should also record this GUID-ValueID map to pass this information to ThinLink phase without VST block.

[mehdi_amini]

The context and the motivation is *still* not clear to me. To begin why do we need the *names* when doing the thin link?

[tejohnson]

In D35189#803332, @mehdi_amini wrote:

The context and the motivation is *still* not clear to me. To begin why do we need the *names* when doing the thin link?

Right - I think we don't need the IRSymtab or strtab with this approach since the valueID->GUID correspondence is being recorded directly. We had talked about emitting the value IDs via the VST and the names via the symtab/strtab, but that doesn't work well (it's not that the VST has redundant info, as Haojie indicated in his comment, but rather that it doesn't have the right info - it no longer maps the value id to the name or even directly to a symtab entry). This issue just affects the summary description of this patch since the actual minimization is being left for a follow-on - Haojie, can you update the summary description of the patch?

lib/Bitcode/Reader/BitcodeReader.cpp

5225

Error string needs update (not just a combined record, but any summary record now)

lib/Bitcode/Writer/BitcodeWriter.cpp

133

This isn't quite right - we are still only generating the value id in this class for guids from indirect call profiles. I would suggest going back to the old description and leaving the map as-is. More on that below.

321

Needs a function description. Note that this needs to be called before we write out the summaries, since we write out the GUID->ValueID records first.

However, I don't think you need to do this at all. There is really no reason to fill up the map with the other GUID->valueId mappings since they can be obtained on the fly when we write the FS_VALUE_GUID records.

I'd suggest instead changing the code that writes the FS_VALUE_GUID records such that it not only walks the map (for the value Ids we have to synthesize here for indirect call profiles), but also does the below Module traversals to emit the rest of them (without ever adding them to the map).

3407

the original name's GUID actually.

[mehdi_amini]

lib/Bitcode/Writer/BitcodeWriter.cpp

3408

Please add here *why* this is done, other the comment is fairly not helpful: it just tells me wha the function is doing.
I think the reason is "This is needed because promoting internal symbol to global requires generating a new GUID from the original name".
That said, unless there is another reason I missed, we could also immediately instead store two GUID for internal symbols: the local and the global.
Also, alternatively and even more simpler I think, we could just always use the global GUID for internal symbols, whether they're promoted or not.

[haojiewang]

In D35189#803332, @mehdi_amini wrote:

The context and the motivation is *still* not clear to me. To begin why do we need the *names* when doing the thin link?

We need the GUID because it is needed when we build the combined index in thin link phase. We need ValueId is because the size of GUID is too large so that we do not want to write it everywhere, so we set a ValueId which is indexing from 0. And we should also set a GUID to ValueId map to record the mapping between them.
We need the original name because it is needed in FunctionImport. So we should pass is to thin link phase.

[haojiewang]

In D35189#803619, @tejohnson wrote:

In D35189#803332, @mehdi_amini wrote:

The context and the motivation is *still* not clear to me. To begin why do we need the *names* when doing the thin link?

Right - I think we don't need the IRSymtab or strtab with this approach since the valueID->GUID correspondence is being recorded directly. We had talked about emitting the value IDs via the VST and the names via the symtab/strtab, but that doesn't work well (it's not that the VST has redundant info, as Haojie indicated in his comment, but rather that it doesn't have the right info - it no longer maps the value id to the name or even directly to a symtab entry). This issue just affects the summary description of this patch since the actual minimization is being left for a follow-on - Haojie, can you update the summary description of the patch?

I think the VST is still holding the right info now. If I understand the code correctly, VST will record the mapping between ValueId and ValueName. Then in thin link phase, GUID and OriginalNameGUID will be computed by ValueName along with linkage and module path, so we can get the ValueID to GUID map, and also the value of OriginalGUID. But the ValueName may be too large(since it is a string), so we can replace it with GUID and OriginalNameGUID(if it is needed).

[tejohnson]

In D35189#803919, @haojiewang wrote:

In D35189#803619, @tejohnson wrote:

In D35189#803332, @mehdi_amini wrote:

The context and the motivation is *still* not clear to me. To begin why do we need the *names* when doing the thin link?

Right - I think we don't need the IRSymtab or strtab with this approach since the valueID->GUID correspondence is being recorded directly. We had talked about emitting the value IDs via the VST and the names via the symtab/strtab, but that doesn't work well (it's not that the VST has redundant info, as Haojie indicated in his comment, but rather that it doesn't have the right info - it no longer maps the value id to the name or even directly to a symtab entry). This issue just affects the summary description of this patch since the actual minimization is being left for a follow-on - Haojie, can you update the summary description of the patch?

I think the VST is still holding the right info now. If I understand the code correctly, VST will record the mapping between ValueId and ValueName. Then in thin link phase, GUID and OriginalNameGUID will be computed by ValueName along with linkage and module path, so we can get the ValueID to GUID map, and also the value of OriginalGUID. But the ValueName may be too large(since it is a string), so we can replace it with GUID and OriginalNameGUID(if it is needed).

Just talked to Haojie offline, but for everyone else following along - the function level VST (for instructions, bbs, etc) still includes the name, but the global VST (for the global values we will have in the index) does not.

[mehdi_amini]

In D35189#803910, @haojiewang wrote:

In D35189#803332, @mehdi_amini wrote:

The context and the motivation is *still* not clear to me. To begin why do we need the *names* when doing the thin link?

We need the GUID because it is needed when we build the combined index in thin link phase. We need ValueId is because the size of GUID is too large so that we do not want to write it everywhere, so we set a ValueId which is indexing from 0. And we should also set a GUID to ValueId map to record the mapping between them.
We need the original name because it is needed in FunctionImport. So we should pass is to thin link phase.

You manage to not really answer fully my question here. But I think infer it from the code in my previous inline comment, it is "we need the name to recompute a GUID for internal symbol promoted to global", correct?

[haojiewang]

In D35189#803953, @mehdi_amini wrote:

In D35189#803910, @haojiewang wrote:

In D35189#803332, @mehdi_amini wrote:

The context and the motivation is *still* not clear to me. To begin why do we need the *names* when doing the thin link?

We need the GUID because it is needed when we build the combined index in thin link phase. We need ValueId is because the size of GUID is too large so that we do not want to write it everywhere, so we set a ValueId which is indexing from 0. And we should also set a GUID to ValueId map to record the mapping between them.
We need the original name because it is needed in FunctionImport. So we should pass is to thin link phase.

You manage to not really answer fully my question here. But I think infer it from the code in my previous inline comment, it is "we need the name to recompute a GUID for internal symbol promoted to global", correct?

Just talked to Teresa, the FunctionImport seems no longer need the original name(but seems that there are still some code in FunctionImport that refer original name), but PGO will use this information.

[tejohnson]

In D35189#803977, @haojiewang wrote:

In D35189#803953, @mehdi_amini wrote:

In D35189#803910, @haojiewang wrote:

In D35189#803332, @mehdi_amini wrote:

The context and the motivation is *still* not clear to me. To begin why do we need the *names* when doing the thin link?

We need the GUID because it is needed when we build the combined index in thin link phase. We need ValueId is because the size of GUID is too large so that we do not want to write it everywhere, so we set a ValueId which is indexing from 0. And we should also set a GUID to ValueId map to record the mapping between them.
We need the original name because it is needed in FunctionImport. So we should pass is to thin link phase.

You manage to not really answer fully my question here. But I think infer it from the code in my previous inline comment, it is "we need the name to recompute a GUID for internal symbol promoted to global", correct?

Just talked to Teresa, the FunctionImport seems no longer need the original name(but seems that there are still some code in FunctionImport that refer original name), but PGO will use this information.

To clarify, I believe Sample PGO is using the original name (as opposed to instrumentation PGO). Dehao?

I am having trouble figuring out how we originally used this information, it was added here D19454 (I believe for the old workaround of not importing from modules with non-renamable values as discussed in D18298, which we now have a better fix for).

[pcc]

the global VST (for the global values we will have in the index) does not.

Not quite, global VSTs include a reference to the symbol name in the strtab.

But I think infer it from the code in my previous inline comment, it is "we need the name to recompute a GUID for internal symbol promoted to global", correct?

Names of globals are also necessary for symbol resolution.

[haojiewang]

Updating D35189. Leave the GUIDToValueIDMap just for indirect call; change the codes that writes the FS_VALUE_GUID records to Module traversals in writePerModuleGlobalValueSummary function; modify some comments.

include/llvm/Bitcode/LLVMBitCodes.h

enum GlobalValueSummarySymtabCodes {
   //                    n x (valueid, hotness)]
   FS_COMBINED_PROFILE = 5,
   // COMBINED_GLOBALVAR_INIT_REFS: [valueid, modid, flags, n x valueid]
   FS_COMBINED_GLOBALVAR_INIT_REFS = 6,
   // ALIAS: [valueid, flags, valueid]
   FS_ALIAS = 7,
   // COMBINED_ALIAS: [valueid, modid, flags, valueid]
   FS_COMBINED_ALIAS = 8,
-  // COMBINED_ORIGINAL_NAME: [original_name_hash]
-  FS_COMBINED_ORIGINAL_NAME = 9,
+  // ORIGINAL_NAME: [original_name_hash]
+  FS_ORIGINAL_NAME = 9,
   // VERSION of the summary, bumped when adding flags for instance.
   FS_VERSION = 10,
   // The list of llvm.type.test type identifiers used by the following function
   // that are used other than by an llvm.assume.
   // [n x typeid]
   FS_TYPE_TESTS = 11,
   // The list of virtual calls made by this function using
   // llvm.assume(llvm.type.test) intrinsics that do not have all constant

lib/Bitcode/Reader/BitcodeReader.cpp

case bitc::VST_CODE_FNENTRY: {
       ValueName.clear();
       break;
     }
     case bitc::VST_CODE_COMBINED_ENTRY: {
       // VST_CODE_COMBINED_ENTRY: [valueid, refguid]
       unsigned ValueID = Record[0];
       GlobalValue::GUID RefGUID = Record[1];
       // The "original name", which is the second value of the pair will be
-      // overriden later by a FS_COMBINED_ORIGINAL_NAME in the combined index.
+      // overriden later by a FS_ORIGINAL_NAME in the combined index.
       ValueIdToValueInfoMap[ValueID] =
           std::make_pair(TheIndex.getOrInsertValueInfo(RefGUID), RefGUID);
       break;
     }
     }
   }
 }
 
case bitc::FS_PERMODULE_PROFILE: {
       PendingTypeTests.clear();
       PendingTypeTestAssumeVCalls.clear();
       PendingTypeCheckedLoadVCalls.clear();
       PendingTypeTestAssumeConstVCalls.clear();
       PendingTypeCheckedLoadConstVCalls.clear();
       auto VIAndOriginalGUID = getValueInfoFromValueId(ValueID);
       FS->setModulePath(addThisModule()->first());
       FS->setOriginalName(VIAndOriginalGUID.second);
+      LastSeenSummary = FS.get();
+      LastSeenGUID = getValueInfoFromValueId(ValueID).first.getGUID();
       TheIndex.addGlobalValueSummary(VIAndOriginalGUID.first, std::move(FS));
       break;
     }
     // FS_ALIAS: [valueid, flags, valueid]
     // Aliases must be emitted (and parsed) after all FS_PERMODULE entries, as
     // they expect all aliasee summaries to be available.
     case bitc::FS_ALIAS: {
       unsigned ValueID = Record[0];
case bitc::FS_ALIAS: {
       auto AliaseeInModule =
           TheIndex.findSummaryInModule(AliaseeGUID, ModulePath);
       if (!AliaseeInModule)
         return error("Alias expects aliasee summary to be parsed");
       AS->setAliasee(AliaseeInModule);
 
       auto GUID = getValueInfoFromValueId(ValueID);
       AS->setOriginalName(GUID.second);
+      LastSeenSummary = AS.get();
+      LastSeenGUID = getValueInfoFromValueId(ValueID).first.getGUID();
       TheIndex.addGlobalValueSummary(GUID.first, std::move(AS));
       break;
     }
     // FS_PERMODULE_GLOBALVAR_INIT_REFS: [valueid, flags, n x valueid]
     case bitc::FS_PERMODULE_GLOBALVAR_INIT_REFS: {
       unsigned ValueID = Record[0];
       uint64_t RawFlags = Record[1];
       auto Flags = getDecodedGVSummaryFlags(RawFlags, Version);
       std::vector<ValueInfo> Refs =
           makeRefList(ArrayRef<uint64_t>(Record).slice(2));
       auto FS = llvm::make_unique<GlobalVarSummary>(Flags, std::move(Refs));
       FS->setModulePath(addThisModule()->first());
       auto GUID = getValueInfoFromValueId(ValueID);
       FS->setOriginalName(GUID.second);
+      LastSeenSummary = FS.get();
+      LastSeenGUID = getValueInfoFromValueId(ValueID).first.getGUID();
       TheIndex.addGlobalValueSummary(GUID.first, std::move(FS));
       break;
     }
     // FS_COMBINED: [valueid, modid, flags, instcount, numrefs,
     //               numrefs x valueid, n x (valueid)]
     // FS_COMBINED_PROFILE: [valueid, modid, flags, instcount, numrefs,
     //                       numrefs x valueid, n x (valueid, hotness)]
     case bitc::FS_COMBINED:
case bitc::FS_COMBINED_GLOBALVAR_INIT_REFS: {
       auto FS = llvm::make_unique<GlobalVarSummary>(Flags, 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;
     }
-    // FS_COMBINED_ORIGINAL_NAME: [original_name]
-    case bitc::FS_COMBINED_ORIGINAL_NAME: {
+    // FS_ORIGINAL_NAME: [original_name]
+    case bitc::FS_ORIGINAL_NAME: {
       uint64_t OriginalName = Record[0];
       if (!LastSeenSummary)
-        return error("Name attachment that does not follow a combined record");
+        return error("Name attachment that does not follow a summary record");
       LastSeenSummary->setOriginalName(OriginalName);
       TheIndex.addOriginalName(LastSeenGUID, OriginalName);
       // Reset the LastSeenSummary
       LastSeenSummary = nullptr;
       LastSeenGUID = 0;
       break;
     }
     case bitc::FS_TYPE_TESTS: {

lib/Bitcode/Writer/BitcodeWriter.cpp

class ModuleBitcodeWriter : public BitcodeWriterBase {
   ModuleHash *ModHash;
 
   /// The start bit of the identification block.
   uint64_t BitcodeStartBit;
 
   /// Map that holds the correspondence between GUIDs in the summary index,
   /// that came from indirect call profiles, and a value id generated by this
   /// class to use in the VST and summary block records.
   std::map<GlobalValue::GUID, unsigned> GUIDToValueIdMap;
 
   /// Tracks the last value id recorded in the GUIDToValueMap.
   unsigned GlobalValueId;
 
   /// Saves the offset of the VSTOffset record that must eventually be
   /// backpatched with the offset of the actual VST.
   uint64_t VSTOffsetPlaceholder = 0;
 
private:
   unsigned getValueId(ValueInfo VI) {
     if (!VI.getValue())
       return getValueId(VI.getGUID());
     return VE.getValueID(VI.getValue());
   }
   std::map<GlobalValue::GUID, unsigned> &valueIds() { return GUIDToValueIdMap; }
 };
 
 /// Class to manage the bitcode writing for a combined index.
 class IndexBitcodeWriter : public BitcodeWriterBase {
   /// The combined index to write to bitcode.
   const ModuleSummaryIndex &Index;
 
   /// When writing a subset of the index for distributed backends, client
   /// provides a map of modules to the corresponding GUIDs/summaries to write.
   const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex;
 
WriteConstVCallVec(bitc::FS_TYPE_CHECKED_LOAD_CONST_VCALL,
                      FS->type_checked_load_const_vcalls());
 }
 
 // Helper to emit a single function summary record.
 void ModuleBitcodeWriter::writePerModuleFunctionSummaryRecord(
     SmallVector<uint64_t, 64> &NameVals, GlobalValueSummary *Summary,
     unsigned ValueID, unsigned FSCallsAbbrev, unsigned FSCallsProfileAbbrev,
     const Function &F) {
+  auto GUID = F.getGUID();
+  Stream.EmitRecord(bitc::FS_VALUE_GUID,
+                      ArrayRef<uint64_t>{ValueID, GUID});
   NameVals.push_back(ValueID);
 
   FunctionSummary *FS = cast<FunctionSummary>(Summary);
   writeFunctionTypeMetadataRecords(Stream, FS);
 
   NameVals.push_back(getEncodedGVSummaryFlags(FS->flags()));
   NameVals.push_back(FS->instCount());
   NameVals.push_back(FS->refs().size());
 
-  for (auto &RI : FS->refs())
-    NameVals.push_back(VE.getValueID(RI.getValue()));
+  for (auto &RI : FS->refs()) {
+    auto RefValueID = VE.getValueID(RI.getValue());
+    auto RefGUID = RI.getGUID();
+    Stream.EmitRecord(bitc::FS_VALUE_GUID,
+                        ArrayRef<uint64_t>{RefValueID, RefGUID});
+    NameVals.push_back(RefValueID);
+  }
 
   bool HasProfileData = F.getEntryCount().hasValue();
   for (auto &ECI : FS->calls()) {
-    NameVals.push_back(getValueId(ECI.first));
+    auto CallValueID = getValueId(ECI.first);
+    auto CallGUID = ECI.first.getGUID();
+    Stream.EmitRecord(bitc::FS_VALUE_GUID,
+                        ArrayRef<uint64_t>{CallValueID, CallGUID});
+    NameVals.push_back(CallValueID);
     if (HasProfileData)
       NameVals.push_back(static_cast<uint8_t>(ECI.second.Hotness));
   }
 
   unsigned FSAbbrev = (HasProfileData ? FSCallsProfileAbbrev : FSCallsAbbrev);
   unsigned Code =
       (HasProfileData ? bitc::FS_PERMODULE_PROFILE : bitc::FS_PERMODULE);
 
void ModuleBitcodeWriter::writeModuleLevelReferences(
   auto VI = Index->getValueInfo(GlobalValue::getGUID(V.getName()));
   if (!VI || VI.getSummaryList().empty()) {
     // Only declarations should not have a summary (a declaration might however
     // have a summary if the def was in module level asm).
     assert(V.isDeclaration());
     return;
   }
   auto *Summary = VI.getSummaryList()[0].get();
-  NameVals.push_back(VE.getValueID(&V));
+  auto ValueID = VE.getValueID(&V);
+  auto GUID = V.getGUID();
+  Stream.EmitRecord(bitc::FS_VALUE_GUID,
+                      ArrayRef<uint64_t>{ValueID, GUID});
+  NameVals.push_back(ValueID);
   GlobalVarSummary *VS = cast<GlobalVarSummary>(Summary);
   NameVals.push_back(getEncodedGVSummaryFlags(VS->flags()));
 
   unsigned SizeBeforeRefs = NameVals.size();
-  for (auto &RI : VS->refs())
-    NameVals.push_back(VE.getValueID(RI.getValue()));
+  for (auto &RI : VS->refs()) {
+    auto RefValueID = VE.getValueID(RI.getValue());
+    auto RefGUID = RI.getGUID();
+    Stream.EmitRecord(bitc::FS_VALUE_GUID,
+                        ArrayRef<uint64_t>{RefValueID, RefGUID});
+    NameVals.push_back(RefValueID);
+  }
   // Sort the refs for determinism output, the vector returned by FS->refs() has
   // been initialized from a DenseSet.
   std::sort(NameVals.begin() + SizeBeforeRefs, NameVals.end());
 
   Stream.EmitRecord(bitc::FS_PERMODULE_GLOBALVAR_INIT_REFS, NameVals,
                     FSModRefsAbbrev);
   NameVals.clear();
 }
void ModuleBitcodeWriter::writePerModuleGlobalValueSummary() {
   Abbv = std::make_shared<BitCodeAbbrev>();
   Abbv->Add(BitCodeAbbrevOp(bitc::FS_ALIAS));
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));   // valueid
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6));   // flags
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));   // valueid
   unsigned FSAliasAbbrev = Stream.EmitAbbrev(std::move(Abbv));
 
   SmallVector<uint64_t, 64> NameVals;
+  // OriginalName's GUID is used in samplePGO, so we add it mseparately
+  // immediately after the record.
+  auto MaybeEmitOriginalName = [&](const GlobalValue &GV) {
+    if (!GlobalValue::isLocalLinkage(GV.getLinkage()))
+      return;
+    NameVals.push_back(GlobalValue::getGUID(GV.getValueName()->getKey()));
+    Stream.EmitRecord(bitc::FS_ORIGINAL_NAME, NameVals);
+    NameVals.clear();
+  };  
   // Iterate over the list of functions instead of the Index to
   // ensure the ordering is stable.
   for (const Function &F : M) {
     // Summary emission does not support anonymous functions, they have to
     // renamed using the anonymous function renaming pass.
     if (!F.hasName())
       report_fatal_error("Unexpected anonymous function when writing summary");
 
     ValueInfo VI = Index->getValueInfo(GlobalValue::getGUID(F.getName()));
     if (!VI || VI.getSummaryList().empty()) {
       // Only declarations should not have a summary (a declaration might
       // however have a summary if the def was in module level asm).
       assert(F.isDeclaration());
       continue;
     }
     auto *Summary = VI.getSummaryList()[0].get();
     writePerModuleFunctionSummaryRecord(NameVals, Summary, VE.getValueID(&F),
                                         FSCallsAbbrev, FSCallsProfileAbbrev, F);
+    MaybeEmitOriginalName(F);
   }
 
   // Capture references from GlobalVariable initializers, which are outside
   // of a function scope.
-  for (const GlobalVariable &G : M.globals())
+  for (const GlobalVariable &G : M.globals()) {
     writeModuleLevelReferences(G, NameVals, FSModRefsAbbrev);
+    MaybeEmitOriginalName(G);
+  }
 
   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 AliasId = VE.getValueID(&A);
     auto AliaseeId = VE.getValueID(Aliasee);
+    auto GUID = A.getGUID();
+    Stream.EmitRecord(bitc::FS_VALUE_GUID,
+                        ArrayRef<uint64_t>{AliasId, GUID});
     NameVals.push_back(AliasId);
     auto *Summary = Index->getGlobalValueSummary(A);
     AliasSummary *AS = cast<AliasSummary>(Summary);
     NameVals.push_back(getEncodedGVSummaryFlags(AS->flags()));
+
+    auto AliaseeGUID = Aliasee->getGUID();
+    Stream.EmitRecord(bitc::FS_VALUE_GUID,
+                        ArrayRef<uint64_t>{AliaseeId, AliaseeGUID});
     NameVals.push_back(AliaseeId);
     Stream.EmitRecord(bitc::FS_ALIAS, NameVals, FSAliasAbbrev);
     NameVals.clear();
+    MaybeEmitOriginalName(A);
   }
 
   Stream.ExitBlock();
 }
 
 /// Emit the combined summary section into the combined index file.
 void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
   Stream.EnterSubblock(bitc::GLOBALVAL_SUMMARY_BLOCK_ID, 3);
void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
   SmallVector<uint64_t, 64> NameVals;
 
   // For local linkage, we also emit the original name separately
   // immediately after the record.
   auto MaybeEmitOriginalName = [&](GlobalValueSummary &S) {
     if (!GlobalValue::isLocalLinkage(S.linkage()))
       return;
     NameVals.push_back(S.getOriginalName());
-    Stream.EmitRecord(bitc::FS_COMBINED_ORIGINAL_NAME, NameVals);
+    Stream.EmitRecord(bitc::FS_ORIGINAL_NAME, NameVals);
     NameVals.clear();
   };
 
   forEachSummary([&](GVInfo I) {
     GlobalValueSummary *S = I.second;
     assert(S);
 
     auto ValueId = getValueId(I.first);

tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp

default:
       STRINGIFY_CODE(FS, PERMODULE)
       STRINGIFY_CODE(FS, PERMODULE_PROFILE)
       STRINGIFY_CODE(FS, PERMODULE_GLOBALVAR_INIT_REFS)
       STRINGIFY_CODE(FS, COMBINED)
       STRINGIFY_CODE(FS, COMBINED_PROFILE)
       STRINGIFY_CODE(FS, COMBINED_GLOBALVAR_INIT_REFS)
       STRINGIFY_CODE(FS, ALIAS)
       STRINGIFY_CODE(FS, COMBINED_ALIAS)
-      STRINGIFY_CODE(FS, COMBINED_ORIGINAL_NAME)
+      STRINGIFY_CODE(FS, ORIGINAL_NAME)
       STRINGIFY_CODE(FS, VERSION)
       STRINGIFY_CODE(FS, TYPE_TESTS)
       STRINGIFY_CODE(FS, TYPE_TEST_ASSUME_VCALLS)
       STRINGIFY_CODE(FS, TYPE_CHECKED_LOAD_VCALLS)
       STRINGIFY_CODE(FS, TYPE_TEST_ASSUME_CONST_VCALL)
       STRINGIFY_CODE(FS, TYPE_CHECKED_LOAD_CONST_VCALL)
       STRINGIFY_CODE(FS, VALUE_GUID)
       STRINGIFY_CODE(FS, CFI_FUNCTION_DEFS)