diff options
author | Benjamin Kramer <benny.kra@googlemail.com> | 2018-03-22 15:29:55 +0000 |
---|---|---|
committer | Benjamin Kramer <benny.kra@googlemail.com> | 2018-03-22 15:29:55 +0000 |
commit | de18a2e6ff8a6c92aeeafa21738c840275da2902 (patch) | |
tree | 29f414205ad617899266fd6ff2631d6be8909421 | |
parent | 9bc0bc4b9b77c3b1fdfcf07477ec560ed0951090 (diff) | |
download | bcm5719-llvm-de18a2e6ff8a6c92aeeafa21738c840275da2902.tar.gz bcm5719-llvm-de18a2e6ff8a6c92aeeafa21738c840275da2902.zip |
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
-rw-r--r-- | llvm/include/llvm/ProfileData/InstrProf.h | 43 | ||||
-rw-r--r-- | llvm/include/llvm/ProfileData/InstrProfReader.h | 2 | ||||
-rw-r--r-- | llvm/lib/ProfileData/InstrProf.cpp | 3 | ||||
-rw-r--r-- | llvm/lib/ProfileData/InstrProfReader.cpp | 29 | ||||
-rw-r--r-- | llvm/lib/ProfileData/InstrProfWriter.cpp | 4 | ||||
-rw-r--r-- | llvm/test/tools/llvm-profdata/invalid-profdata.test | 50 | ||||
-rw-r--r-- | llvm/unittests/ProfileData/InstrProfTest.cpp | 3 |
7 files changed, 30 insertions, 104 deletions
diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h index 88ae0f05e7d..cd3675c7cfb 100644 --- a/llvm/include/llvm/ProfileData/InstrProf.h +++ b/llvm/include/llvm/ProfileData/InstrProf.h @@ -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, diff --git a/llvm/include/llvm/ProfileData/InstrProfReader.h b/llvm/include/llvm/ProfileData/InstrProfReader.h index efc22dcd0d9..813a17dd216 100644 --- a/llvm/include/llvm/ProfileData/InstrProfReader.h +++ b/llvm/include/llvm/ProfileData/InstrProfReader.h @@ -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); } diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp index fd25728a8a8..106b3770cb0 100644 --- a/llvm/lib/ProfileData/InstrProf.cpp +++ b/llvm/lib/ProfileData/InstrProf.cpp @@ -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(); } diff --git a/llvm/lib/ProfileData/InstrProfReader.cpp b/llvm/lib/ProfileData/InstrProfReader.cpp index 64f98ad0051..b920beeb1c0 100644 --- a/llvm/lib/ProfileData/InstrProfReader.cpp +++ b/llvm/lib/ProfileData/InstrProfReader.cpp @@ -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(); diff --git a/llvm/lib/ProfileData/InstrProfWriter.cpp b/llvm/lib/ProfileData/InstrProfWriter.cpp index 33ceb66fd26..ce3f8806e12 100644 --- a/llvm/lib/ProfileData/InstrProfWriter.cpp +++ b/llvm/lib/ProfileData/InstrProfWriter.cpp @@ -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())) diff --git a/llvm/test/tools/llvm-profdata/invalid-profdata.test b/llvm/test/tools/llvm-profdata/invalid-profdata.test deleted file mode 100644 index 4d6936f8a32..00000000000 --- a/llvm/test/tools/llvm-profdata/invalid-profdata.test +++ /dev/null @@ -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 diff --git a/llvm/unittests/ProfileData/InstrProfTest.cpp b/llvm/unittests/ProfileData/InstrProfTest.cpp index 09be2e3dad7..432d91464b2 100644 --- a/llvm/unittests/ProfileData/InstrProfTest.cpp +++ b/llvm/unittests/ProfileData/InstrProfTest.cpp @@ -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")); |