Skip to content

Commit

Permalink
[ThinLTO] Add string saver onto index for value names
Browse files Browse the repository at this point in the history
Summary:
Adds a string saver to the ModuleSummaryIndex so it can store value
names in the case of adding a ValueInfo for a GUID when we don't
have the name stored in a Module string table. This is motivated
by the upcoming summary parser patch, where we will read value names
from the summary entry and want to store them, even when a Module
is not available.

Currently this allows us to store the name in the legacy bitcode case,
and I have added a test to show that.

Reviewers: pcc, dexonsmith

Subscribers: mehdi_amini, inglorion, eraman, steven_wu, llvm-commits

Differential Revision: https://reviews.llvm.org/D47842

llvm-svn: 335570
  • Loading branch information
teresajohnson committed Jun 26, 2018
1 parent e44acad commit 5190553
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 6 deletions.
14 changes: 13 additions & 1 deletion llvm/include/llvm/IR/ModuleSummaryIndex.h
Expand Up @@ -25,8 +25,10 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/IR/GlobalValue.h"
#include "llvm/IR/Module.h"
#include "llvm/Support/Allocator.h"
#include "llvm/Support/MathExtras.h"
#include "llvm/Support/ScaledNumber.h"
#include "llvm/Support/StringSaver.h"
#include <algorithm>
#include <array>
#include <cassert>
Expand Down Expand Up @@ -766,6 +768,11 @@ class ModuleSummaryIndex {
std::set<std::string> CfiFunctionDefs;
std::set<std::string> CfiFunctionDecls;

// Used in cases where we want to record the name of a global, but
// don't have the string owned elsewhere (e.g. the Strtab on a module).
StringSaver Saver;
BumpPtrAllocator Alloc;

// YAML I/O support.
friend yaml::MappingTraits<ModuleSummaryIndex>;

Expand All @@ -777,7 +784,7 @@ class ModuleSummaryIndex {

public:
// See HaveGVs variable comment.
ModuleSummaryIndex(bool HaveGVs) : HaveGVs(HaveGVs) {}
ModuleSummaryIndex(bool HaveGVs) : HaveGVs(HaveGVs), Saver(Alloc) {}

bool haveGVs() const { return HaveGVs; }

Expand Down Expand Up @@ -886,6 +893,11 @@ class ModuleSummaryIndex {
return ValueInfo(HaveGVs, getOrInsertValuePtr(GUID));
}

// Save a string in the Index. Use before passing Name to
// getOrInsertValueInfo when the string isn't owned elsewhere (e.g. on the
// module's Strtab).
StringRef saveString(std::string String) { return Saver.save(String); }

/// Return a ValueInfo for \p GUID setting value \p Name.
ValueInfo getOrInsertValueInfo(GlobalValue::GUID GUID, StringRef Name) {
assert(!HaveGVs);
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
Expand Up @@ -607,14 +607,14 @@ ModuleSummaryIndexWrapperPass::ModuleSummaryIndexWrapperPass()

bool ModuleSummaryIndexWrapperPass::runOnModule(Module &M) {
auto &PSI = *getAnalysis<ProfileSummaryInfoWrapperPass>().getPSI();
Index = buildModuleSummaryIndex(
Index.emplace(buildModuleSummaryIndex(
M,
[this](const Function &F) {
return &(this->getAnalysis<BlockFrequencyInfoWrapperPass>(
*const_cast<Function *>(&F))
.getBFI());
},
&PSI);
&PSI));
return false;
}

Expand Down
9 changes: 6 additions & 3 deletions llvm/lib/Bitcode/Reader/BitcodeReader.cpp
Expand Up @@ -4836,11 +4836,14 @@ void ModuleSummaryIndexBitcodeReader::setValueGUID(
if (PrintSummaryGUIDs)
dbgs() << "GUID " << ValueGUID << "(" << OriginalNameID << ") is "
<< ValueName << "\n";

// UseStrtab is false for legacy summary formats and value names are
// created on stack. We can't use them outside of parseValueSymbolTable.
// created on stack. In that case we save the name in a string saver in
// the index so that the value name can be recorded.
ValueIdToValueInfoMap[ValueID] = std::make_pair(
TheIndex.getOrInsertValueInfo(ValueGUID, UseStrtab ? ValueName : ""),
TheIndex.getOrInsertValueInfo(
ValueGUID,
UseStrtab ? ValueName : TheIndex.saveString(ValueName.str())),
OriginalNameID);
}

Expand Down
7 changes: 7 additions & 0 deletions llvm/test/ThinLTO/X86/autoupgrade.ll
Expand Up @@ -12,6 +12,13 @@
; CHECK: <STRTAB_BLOCK
; CHECK-NEXT: blob data = 'mainglobalfunc1llvm.invariant.start.p0i8{{.*}}'

; Check that the summary is able to print the names despite the lack of
; string table in the legacy bitcode.
; RUN: llvm-dis %p/Inputs/autoupgrade.bc -o - \
; RUN: | FileCheck %s --check-prefix=SUMMARYNAMES
; SUMMARYNAMES: ^2 = gv: (name: "globalfunc2",
; SUMMARYNAMES: ^3 = gv: (name: "globalfunc1"

target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-apple-macosx10.11.0"

Expand Down

0 comments on commit 5190553

Please sign in to comment.