Revert "[InstrProf] Support for external functions in text format."
This reverts commit r328132. Breaks FDO selfhost. I'm seeing error: /tmp/profraw: Invalid instrumentation profile data (bad magic) llvm-svn: 328207
This commit is contained in:
parent
9bc0bc4b9b
commit
de18a2e6ff
|
@ -425,17 +425,6 @@ private:
|
|||
// A map from function runtime address to function name MD5 hash.
|
||||
// This map is only populated and used by raw instr profile reader.
|
||||
AddrHashMap AddrToMD5Map;
|
||||
bool Sorted = false;
|
||||
|
||||
static StringRef getExternalSymbol() {
|
||||
return "** External Symbol **";
|
||||
}
|
||||
|
||||
// If the symtab is created by a series of calls to \c addFuncName, \c
|
||||
// finalizeSymtab needs to be called before looking up function names.
|
||||
// This is required because the underlying map is a vector (for space
|
||||
// efficiency) which needs to be sorted.
|
||||
inline void finalizeSymtab();
|
||||
|
||||
public:
|
||||
InstrProfSymtab() = default;
|
||||
|
@ -467,17 +456,21 @@ public:
|
|||
/// \p IterRange. This interface is used by IndexedProfReader.
|
||||
template <typename NameIterRange> Error create(const NameIterRange &IterRange);
|
||||
|
||||
// If the symtab is created by a series of calls to \c addFuncName, \c
|
||||
// finalizeSymtab needs to be called before looking up function names.
|
||||
// This is required because the underlying map is a vector (for space
|
||||
// efficiency) which needs to be sorted.
|
||||
inline void finalizeSymtab();
|
||||
|
||||
/// Update the symtab by adding \p FuncName to the table. This interface
|
||||
/// is used by the raw and text profile readers.
|
||||
Error addFuncName(StringRef FuncName) {
|
||||
if (FuncName.empty())
|
||||
return make_error<InstrProfError>(instrprof_error::malformed);
|
||||
auto Ins = NameTab.insert(FuncName);
|
||||
if (Ins.second) {
|
||||
if (Ins.second)
|
||||
MD5NameMap.push_back(std::make_pair(
|
||||
IndexedInstrProf::ComputeHash(FuncName), Ins.first->getKey()));
|
||||
Sorted = false;
|
||||
}
|
||||
return Error::success();
|
||||
}
|
||||
|
||||
|
@ -499,16 +492,6 @@ public:
|
|||
/// If not found, return an empty string.
|
||||
inline StringRef getFuncName(uint64_t FuncMD5Hash);
|
||||
|
||||
/// Just like getFuncName, except that it will return a non-empty StringRef
|
||||
/// if the function is external to this symbol table. All such cases
|
||||
/// will be represented using the same StringRef value.
|
||||
inline StringRef getFuncNameOrExternalSymbol(uint64_t FuncMD5Hash);
|
||||
|
||||
/// True if Symbol is the value used to represent external symbols.
|
||||
static bool isExternalSymbol(const StringRef &Symbol) {
|
||||
return Symbol == InstrProfSymtab::getExternalSymbol();
|
||||
}
|
||||
|
||||
/// Return function from the name's md5 hash. Return nullptr if not found.
|
||||
inline Function *getFunction(uint64_t FuncMD5Hash);
|
||||
|
||||
|
@ -542,25 +525,14 @@ Error InstrProfSymtab::create(const NameIterRange &IterRange) {
|
|||
}
|
||||
|
||||
void InstrProfSymtab::finalizeSymtab() {
|
||||
if (Sorted)
|
||||
return;
|
||||
std::sort(MD5NameMap.begin(), MD5NameMap.end(), less_first());
|
||||
std::sort(MD5FuncMap.begin(), MD5FuncMap.end(), less_first());
|
||||
std::sort(AddrToMD5Map.begin(), AddrToMD5Map.end(), less_first());
|
||||
AddrToMD5Map.erase(std::unique(AddrToMD5Map.begin(), AddrToMD5Map.end()),
|
||||
AddrToMD5Map.end());
|
||||
Sorted = true;
|
||||
}
|
||||
|
||||
StringRef InstrProfSymtab::getFuncNameOrExternalSymbol(uint64_t FuncMD5Hash) {
|
||||
StringRef ret = getFuncName(FuncMD5Hash);
|
||||
if (ret.empty())
|
||||
return InstrProfSymtab::getExternalSymbol();
|
||||
return ret;
|
||||
}
|
||||
|
||||
StringRef InstrProfSymtab::getFuncName(uint64_t FuncMD5Hash) {
|
||||
finalizeSymtab();
|
||||
auto Result =
|
||||
std::lower_bound(MD5NameMap.begin(), MD5NameMap.end(), FuncMD5Hash,
|
||||
[](const std::pair<uint64_t, std::string> &LHS,
|
||||
|
@ -571,7 +543,6 @@ StringRef InstrProfSymtab::getFuncName(uint64_t FuncMD5Hash) {
|
|||
}
|
||||
|
||||
Function* InstrProfSymtab::getFunction(uint64_t FuncMD5Hash) {
|
||||
finalizeSymtab();
|
||||
auto Result =
|
||||
std::lower_bound(MD5FuncMap.begin(), MD5FuncMap.end(), FuncMD5Hash,
|
||||
[](const std::pair<uint64_t, Function*> &LHS,
|
||||
|
|
|
@ -101,7 +101,7 @@ protected:
|
|||
return make_error<InstrProfError>(Err);
|
||||
}
|
||||
|
||||
Error error(Error &&E) { return error(InstrProfError::take(std::move(E))); }
|
||||
Error error(Error E) { return error(InstrProfError::take(std::move(E))); }
|
||||
|
||||
/// Clear the current error and return a successful one.
|
||||
Error success() { return error(instrprof_error::success); }
|
||||
|
|
|
@ -355,7 +355,7 @@ Error InstrProfSymtab::create(Module &M, bool InLTO) {
|
|||
}
|
||||
}
|
||||
}
|
||||
Sorted = false;
|
||||
|
||||
finalizeSymtab();
|
||||
return Error::success();
|
||||
}
|
||||
|
@ -476,6 +476,7 @@ Error readPGOFuncNameStrings(StringRef NameStrings, InstrProfSymtab &Symtab) {
|
|||
while (P < EndP && *P == 0)
|
||||
P++;
|
||||
}
|
||||
Symtab.finalizeSymtab();
|
||||
return Error::success();
|
||||
}
|
||||
|
||||
|
|
|
@ -200,13 +200,9 @@ TextInstrProfReader::readValueProfileData(InstrProfRecord &Record) {
|
|||
std::pair<StringRef, StringRef> VD = Line->rsplit(':');
|
||||
uint64_t TakenCount, Value;
|
||||
if (ValueKind == IPVK_IndirectCallTarget) {
|
||||
if (InstrProfSymtab::isExternalSymbol(VD.first)) {
|
||||
Value = 0;
|
||||
} else {
|
||||
if (Error E = Symtab->addFuncName(VD.first))
|
||||
return E;
|
||||
Value = IndexedInstrProf::ComputeHash(VD.first);
|
||||
}
|
||||
if (Error E = Symtab->addFuncName(VD.first))
|
||||
return E;
|
||||
Value = IndexedInstrProf::ComputeHash(VD.first);
|
||||
} else {
|
||||
READ_NUM(VD.first, Value);
|
||||
}
|
||||
|
@ -231,13 +227,14 @@ Error TextInstrProfReader::readNextRecord(NamedInstrProfRecord &Record) {
|
|||
++Line;
|
||||
// If we hit EOF while looking for a name, we're done.
|
||||
if (Line.is_at_end()) {
|
||||
Symtab->finalizeSymtab();
|
||||
return error(instrprof_error::eof);
|
||||
}
|
||||
|
||||
// Read the function name.
|
||||
Record.Name = *Line++;
|
||||
if (Error E = Symtab->addFuncName(Record.Name))
|
||||
return error(std::move(E));
|
||||
return E;
|
||||
|
||||
// Read the function hash.
|
||||
if (Line.is_at_end())
|
||||
|
@ -268,8 +265,11 @@ Error TextInstrProfReader::readNextRecord(NamedInstrProfRecord &Record) {
|
|||
|
||||
// Check if value profile data exists and read it if so.
|
||||
if (Error E = readValueProfileData(Record))
|
||||
return error(std::move(E));
|
||||
return E;
|
||||
|
||||
// This is needed to avoid two pass parsing because llvm-profdata
|
||||
// does dumping while reading.
|
||||
Symtab->finalizeSymtab();
|
||||
return success();
|
||||
}
|
||||
|
||||
|
@ -331,6 +331,7 @@ Error RawInstrProfReader<IntPtrT>::createSymtab(InstrProfSymtab &Symtab) {
|
|||
continue;
|
||||
Symtab.mapAddress(FPtr, I->NameRef);
|
||||
}
|
||||
Symtab.finalizeSymtab();
|
||||
return success();
|
||||
}
|
||||
|
||||
|
@ -448,23 +449,23 @@ Error RawInstrProfReader<IntPtrT>::readNextRecord(NamedInstrProfRecord &Record)
|
|||
if (atEnd())
|
||||
// At this point, ValueDataStart field points to the next header.
|
||||
if (Error E = readNextHeader(getNextHeaderPos()))
|
||||
return error(std::move(E));
|
||||
return E;
|
||||
|
||||
// Read name ad set it in Record.
|
||||
if (Error E = readName(Record))
|
||||
return error(std::move(E));
|
||||
return E;
|
||||
|
||||
// Read FuncHash and set it in Record.
|
||||
if (Error E = readFuncHash(Record))
|
||||
return error(std::move(E));
|
||||
return E;
|
||||
|
||||
// Read raw counts and set Record.
|
||||
if (Error E = readRawCounts(Record))
|
||||
return error(std::move(E));
|
||||
return E;
|
||||
|
||||
// Read value data and set Record.
|
||||
if (Error E = readValueProfilingData(Record))
|
||||
return error(std::move(E));
|
||||
return E;
|
||||
|
||||
// Iterate.
|
||||
advanceData();
|
||||
|
|
|
@ -361,8 +361,7 @@ void InstrProfWriter::writeRecordInText(StringRef Name, uint64_t Hash,
|
|||
std::unique_ptr<InstrProfValueData[]> VD = Func.getValueForSite(VK, S);
|
||||
for (uint32_t I = 0; I < ND; I++) {
|
||||
if (VK == IPVK_IndirectCallTarget)
|
||||
OS << Symtab.getFuncNameOrExternalSymbol(VD[I].Value) << ":"
|
||||
<< VD[I].Count << "\n";
|
||||
OS << Symtab.getFuncName(VD[I].Value) << ":" << VD[I].Count << "\n";
|
||||
else
|
||||
OS << VD[I].Value << ":" << VD[I].Count << "\n";
|
||||
}
|
||||
|
@ -380,6 +379,7 @@ Error InstrProfWriter::writeText(raw_fd_ostream &OS) {
|
|||
if (shouldEncodeData(I.getValue()))
|
||||
if (Error E = Symtab.addFuncName(I.getKey()))
|
||||
return E;
|
||||
Symtab.finalizeSymtab();
|
||||
|
||||
for (const auto &I : FunctionData)
|
||||
if (shouldEncodeData(I.getValue()))
|
||||
|
|
|
@ -1,50 +0,0 @@
|
|||
RUN: echo ":ir" > %t.input
|
||||
RUN: echo "_ZN6Thread5StartEv" >> %t.input
|
||||
RUN: echo "# Func Hash:" >> %t.input
|
||||
RUN: echo "288793635542036872" >> %t.input
|
||||
RUN: echo "# Num Counters:" >> %t.input
|
||||
RUN: echo "3" >> %t.input
|
||||
RUN: echo "# Counter Values:" >> %t.input
|
||||
RUN: echo "0" >> %t.input
|
||||
RUN: echo "12" >> %t.input
|
||||
RUN: echo "12" >> %t.input
|
||||
RUN: echo "# Num Value Kinds:" >> %t.input
|
||||
RUN: echo "1" >> %t.input
|
||||
RUN: echo "# ValueKind = IPVK_IndirectCallTarget:" >> %t.input
|
||||
RUN: echo "0" >> %t.input
|
||||
RUN: echo "# NumValueSites:" >> %t.input
|
||||
RUN: echo "2" >> %t.input
|
||||
RUN: echo "2" >> %t.input
|
||||
RUN: echo "f1:10" >> %t.input
|
||||
RUN: echo "f2:0" >> %t.input
|
||||
RUN: echo "1" >> %t.input
|
||||
RUN: echo ":10" >> %t.input
|
||||
|
||||
RUN: not llvm-profdata merge %t.input -text -o /dev/null 2>&1 | FileCheck %s --check-prefix=BROKEN
|
||||
BROKEN: Malformed instrumentation profile data
|
||||
|
||||
RUN: echo ":ir" > %t.input
|
||||
RUN: echo "_ZN6Thread5StartEv" >> %t.input
|
||||
RUN: echo "# Func Hash:" >> %t.input
|
||||
RUN: echo "288793635542036872" >> %t.input
|
||||
RUN: echo "# Num Counters:" >> %t.input
|
||||
RUN: echo "3" >> %t.input
|
||||
RUN: echo "# Counter Values:" >> %t.input
|
||||
RUN: echo "0" >> %t.input
|
||||
RUN: echo "12" >> %t.input
|
||||
RUN: echo "12" >> %t.input
|
||||
RUN: echo "# Num Value Kinds:" >> %t.input
|
||||
RUN: echo "1" >> %t.input
|
||||
RUN: echo "# ValueKind = IPVK_IndirectCallTarget:" >> %t.input
|
||||
RUN: echo "0" >> %t.input
|
||||
RUN: echo "# NumValueSites:" >> %t.input
|
||||
RUN: echo "2" >> %t.input
|
||||
RUN: echo "2" >> %t.input
|
||||
RUN: echo "f1:10" >> %t.input
|
||||
RUN: echo "f2:0" >> %t.input
|
||||
RUN: echo "1" >> %t.input
|
||||
RUN: echo "** External Symbol **:10" >> %t.input
|
||||
|
||||
# RUN: llvm-profdata merge %t.input -text -output=%t.out && cat %t.out | FileCheck %s
|
||||
|
||||
CHECK: ** External Symbol **:10
|
|
@ -769,6 +769,7 @@ TEST_P(MaybeSparseInstrProfTest, value_prof_data_read_write_mapping) {
|
|||
Symtab.mapAddress(uint64_t(callee3), 0x3000ULL);
|
||||
Symtab.mapAddress(uint64_t(callee4), 0x4000ULL);
|
||||
// Missing mapping for callee5
|
||||
Symtab.finalizeSymtab();
|
||||
|
||||
VPData->deserializeTo(Record, &Symtab);
|
||||
|
||||
|
@ -857,6 +858,8 @@ TEST_P(MaybeSparseInstrProfTest, instr_prof_symtab_test) {
|
|||
EXPECT_THAT_ERROR(Symtab.addFuncName("blah_1"), Succeeded());
|
||||
EXPECT_THAT_ERROR(Symtab.addFuncName("blah_2"), Succeeded());
|
||||
EXPECT_THAT_ERROR(Symtab.addFuncName("blah_3"), Succeeded());
|
||||
// Finalize it
|
||||
Symtab.finalizeSymtab();
|
||||
|
||||
// Check again
|
||||
R = Symtab.getFuncName(IndexedInstrProf::ComputeHash("blah_1"));
|
||||
|
|
Loading…
Reference in New Issue