[llvm] r328244 - Revert "Revert "[InstrProf] Support for external functions in text format.""
Mircea Trofin via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 22 14:26:52 PDT 2018
Author: mtrofin
Date: Thu Mar 22 14:26:52 2018
New Revision: 328244
URL: http://llvm.org/viewvc/llvm-project?rev=328244&view=rev
Log:
Revert "Revert "[InstrProf] Support for external functions in text format.""
Summary:
This reverts commit 364eb09576a7667bc6d3ff80c52a83014ccac976 and separates out
the portion that was fixing binary reader error propagation - turns out, there
are production cases where that causes a regression.
Will re-introduce the error propagation fix separately.
The fix to the text reader error propagation is still "in".
Reviewers: bkramer
Reviewed By: bkramer
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D44807
Added:
llvm/trunk/test/tools/llvm-profdata/invalid-profdata.test
Modified:
llvm/trunk/include/llvm/ProfileData/InstrProf.h
llvm/trunk/include/llvm/ProfileData/InstrProfReader.h
llvm/trunk/lib/ProfileData/InstrProf.cpp
llvm/trunk/lib/ProfileData/InstrProfReader.cpp
llvm/trunk/lib/ProfileData/InstrProfWriter.cpp
llvm/trunk/unittests/ProfileData/InstrProfTest.cpp
Modified: llvm/trunk/include/llvm/ProfileData/InstrProf.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ProfileData/InstrProf.h?rev=328244&r1=328243&r2=328244&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ProfileData/InstrProf.h (original)
+++ llvm/trunk/include/llvm/ProfileData/InstrProf.h Thu Mar 22 14:26:52 2018
@@ -425,6 +425,17 @@ 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;
@@ -456,21 +467,17 @@ 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();
}
@@ -492,6 +499,16 @@ 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);
@@ -525,14 +542,25 @@ Error InstrProfSymtab::create(const Name
}
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,
@@ -543,6 +571,7 @@ StringRef InstrProfSymtab::getFuncName(u
}
Function* InstrProfSymtab::getFunction(uint64_t FuncMD5Hash) {
+ finalizeSymtab();
auto Result =
std::lower_bound(MD5FuncMap.begin(), MD5FuncMap.end(), FuncMD5Hash,
[](const std::pair<uint64_t, Function*> &LHS,
Modified: llvm/trunk/include/llvm/ProfileData/InstrProfReader.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ProfileData/InstrProfReader.h?rev=328244&r1=328243&r2=328244&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ProfileData/InstrProfReader.h (original)
+++ llvm/trunk/include/llvm/ProfileData/InstrProfReader.h Thu Mar 22 14:26:52 2018
@@ -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); }
Modified: llvm/trunk/lib/ProfileData/InstrProf.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ProfileData/InstrProf.cpp?rev=328244&r1=328243&r2=328244&view=diff
==============================================================================
--- llvm/trunk/lib/ProfileData/InstrProf.cpp (original)
+++ llvm/trunk/lib/ProfileData/InstrProf.cpp Thu Mar 22 14:26:52 2018
@@ -355,7 +355,7 @@ Error InstrProfSymtab::create(Module &M,
}
}
}
-
+ Sorted = false;
finalizeSymtab();
return Error::success();
}
@@ -476,7 +476,6 @@ Error readPGOFuncNameStrings(StringRef N
while (P < EndP && *P == 0)
P++;
}
- Symtab.finalizeSymtab();
return Error::success();
}
Modified: llvm/trunk/lib/ProfileData/InstrProfReader.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ProfileData/InstrProfReader.cpp?rev=328244&r1=328243&r2=328244&view=diff
==============================================================================
--- llvm/trunk/lib/ProfileData/InstrProfReader.cpp (original)
+++ llvm/trunk/lib/ProfileData/InstrProfReader.cpp Thu Mar 22 14:26:52 2018
@@ -200,9 +200,13 @@ TextInstrProfReader::readValueProfileDat
std::pair<StringRef, StringRef> VD = Line->rsplit(':');
uint64_t TakenCount, Value;
if (ValueKind == IPVK_IndirectCallTarget) {
- if (Error E = Symtab->addFuncName(VD.first))
- return E;
- Value = IndexedInstrProf::ComputeHash(VD.first);
+ if (InstrProfSymtab::isExternalSymbol(VD.first)) {
+ Value = 0;
+ } else {
+ if (Error E = Symtab->addFuncName(VD.first))
+ return E;
+ Value = IndexedInstrProf::ComputeHash(VD.first);
+ }
} else {
READ_NUM(VD.first, Value);
}
@@ -227,14 +231,13 @@ Error TextInstrProfReader::readNextRecor
++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 E;
+ return error(std::move(E));
// Read the function hash.
if (Line.is_at_end())
@@ -265,11 +268,8 @@ Error TextInstrProfReader::readNextRecor
// Check if value profile data exists and read it if so.
if (Error E = readValueProfileData(Record))
- return E;
+ return error(std::move(E));
- // This is needed to avoid two pass parsing because llvm-profdata
- // does dumping while reading.
- Symtab->finalizeSymtab();
return success();
}
@@ -331,7 +331,6 @@ Error RawInstrProfReader<IntPtrT>::creat
continue;
Symtab.mapAddress(FPtr, I->NameRef);
}
- Symtab.finalizeSymtab();
return success();
}
Modified: llvm/trunk/lib/ProfileData/InstrProfWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ProfileData/InstrProfWriter.cpp?rev=328244&r1=328243&r2=328244&view=diff
==============================================================================
--- llvm/trunk/lib/ProfileData/InstrProfWriter.cpp (original)
+++ llvm/trunk/lib/ProfileData/InstrProfWriter.cpp Thu Mar 22 14:26:52 2018
@@ -361,7 +361,8 @@ void InstrProfWriter::writeRecordInText(
std::unique_ptr<InstrProfValueData[]> VD = Func.getValueForSite(VK, S);
for (uint32_t I = 0; I < ND; I++) {
if (VK == IPVK_IndirectCallTarget)
- OS << Symtab.getFuncName(VD[I].Value) << ":" << VD[I].Count << "\n";
+ OS << Symtab.getFuncNameOrExternalSymbol(VD[I].Value) << ":"
+ << VD[I].Count << "\n";
else
OS << VD[I].Value << ":" << VD[I].Count << "\n";
}
@@ -379,7 +380,6 @@ Error InstrProfWriter::writeText(raw_fd_
if (shouldEncodeData(I.getValue()))
if (Error E = Symtab.addFuncName(I.getKey()))
return E;
- Symtab.finalizeSymtab();
for (const auto &I : FunctionData)
if (shouldEncodeData(I.getValue()))
Added: llvm/trunk/test/tools/llvm-profdata/invalid-profdata.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-profdata/invalid-profdata.test?rev=328244&view=auto
==============================================================================
--- llvm/trunk/test/tools/llvm-profdata/invalid-profdata.test (added)
+++ llvm/trunk/test/tools/llvm-profdata/invalid-profdata.test Thu Mar 22 14:26:52 2018
@@ -0,0 +1,50 @@
+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
Modified: llvm/trunk/unittests/ProfileData/InstrProfTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ProfileData/InstrProfTest.cpp?rev=328244&r1=328243&r2=328244&view=diff
==============================================================================
--- llvm/trunk/unittests/ProfileData/InstrProfTest.cpp (original)
+++ llvm/trunk/unittests/ProfileData/InstrProfTest.cpp Thu Mar 22 14:26:52 2018
@@ -769,7 +769,6 @@ TEST_P(MaybeSparseInstrProfTest, value_p
Symtab.mapAddress(uint64_t(callee3), 0x3000ULL);
Symtab.mapAddress(uint64_t(callee4), 0x4000ULL);
// Missing mapping for callee5
- Symtab.finalizeSymtab();
VPData->deserializeTo(Record, &Symtab);
@@ -858,8 +857,6 @@ TEST_P(MaybeSparseInstrProfTest, instr_p
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"));
More information about the llvm-commits
mailing list