diff options
author | Vedant Kumar <vsk@apple.com> | 2016-03-28 20:12:07 +0000 |
---|---|---|
committer | Vedant Kumar <vsk@apple.com> | 2016-03-28 20:12:07 +0000 |
commit | f20b6cec1c50d624fb5fb6a9003407be6e9f1bad (patch) | |
tree | 72f1570255b05eba3bbcaac3faaf05b41c4b89f4 | |
parent | 0d026d9e9eb4fa1695dc94be43f02579f59bc696 (diff) | |
download | bcm5719-llvm-f20b6cec1c50d624fb5fb6a9003407be6e9f1bad.tar.gz bcm5719-llvm-f20b6cec1c50d624fb5fb6a9003407be6e9f1bad.zip |
Reapply "[PGO] Fix name encoding for ObjC-like functions"
Function names in ObjC can have spaces in them. This interacts poorly
with name compression, which uses spaces to separate PGO names. Fix the
issue by using a different separator and update a test.
I chose "\01" as the separator because 1) it's non-printable, 2) we
strip it from PGO names, and 3) it's the next natural choice once "\00"
is discarded (that one's overloaded).
This reverts the revert commit beaf3d18. What's changed?
- I fixed up the covmap-V2 binary format tests using a linux VM.
- I updated the expected counts in instrprof-comdat.h to account for
the fact that there have been bugfixes to clang coverage.
- I added an assert to make sure we don't get bitten by this again.
Differential Revision: http://reviews.llvm.org/D18516
llvm-svn: 264641
-rw-r--r-- | llvm/include/llvm/ProfileData/InstrProf.h | 3 | ||||
-rw-r--r-- | llvm/lib/ProfileData/InstrProf.cpp | 10 | ||||
-rwxr-xr-x | llvm/test/tools/llvm-cov/Inputs/binary-formats.v2.linux32l | bin | 29634 -> 29424 bytes | |||
-rwxr-xr-x | llvm/test/tools/llvm-cov/Inputs/binary-formats.v2.linux64l | bin | 2125077 -> 2124848 bytes | |||
-rw-r--r-- | llvm/test/tools/llvm-cov/Inputs/elf_binary_comdat.profdata | bin | 1040 -> 840 bytes | |||
-rw-r--r-- | llvm/test/tools/llvm-cov/Inputs/instrprof-comdat.h | 11 | ||||
-rw-r--r-- | llvm/test/tools/llvm-profdata/Inputs/c-general.profraw | bin | 1696 -> 1648 bytes | |||
-rw-r--r-- | llvm/test/tools/llvm-profdata/raw-32-bits-be.test | 2 | ||||
-rw-r--r-- | llvm/test/tools/llvm-profdata/raw-32-bits-le.test | 2 | ||||
-rw-r--r-- | llvm/test/tools/llvm-profdata/raw-64-bits-be.test | 2 | ||||
-rw-r--r-- | llvm/test/tools/llvm-profdata/raw-64-bits-le.test | 2 | ||||
-rw-r--r-- | llvm/unittests/ProfileData/InstrProfTest.cpp | 4 |
12 files changed, 25 insertions, 11 deletions
diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h index 3bf0aa38510..31fcffe2ef7 100644 --- a/llvm/include/llvm/ProfileData/InstrProf.h +++ b/llvm/include/llvm/ProfileData/InstrProf.h @@ -151,6 +151,9 @@ inline StringRef getInstrProfFileOverriderFuncName() { return "__llvm_profile_override_default_filename"; } +/// Return the marker used to separate PGO names during serialization. +inline StringRef getInstrProfNameSeparator() { return "\01"; } + /// Return the modified name for function \c F suitable to be /// used the key for profile lookup. std::string getPGOFuncName(const Function &F, diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp index 2446521a5bc..1be3df4699c 100644 --- a/llvm/lib/ProfileData/InstrProf.cpp +++ b/llvm/lib/ProfileData/InstrProf.cpp @@ -158,9 +158,15 @@ void InstrProfSymtab::create(const Module &M) { int collectPGOFuncNameStrings(const std::vector<std::string> &NameStrs, bool doCompression, std::string &Result) { + assert(NameStrs.size() && "No name data to emit"); + uint8_t Header[16], *P = Header; std::string UncompressedNameStrings = - join(NameStrs.begin(), NameStrs.end(), StringRef(" ")); + join(NameStrs.begin(), NameStrs.end(), getInstrProfNameSeparator()); + + assert(StringRef(UncompressedNameStrings) + .count(getInstrProfNameSeparator()) == (NameStrs.size() - 1) && + "PGO name is invalid (contains separator token)"); unsigned EncLen = encodeULEB128(UncompressedNameStrings.length(), P); P += EncLen; @@ -238,7 +244,7 @@ int readPGOFuncNameStrings(StringRef NameStrings, InstrProfSymtab &Symtab) { } // Now parse the name strings. SmallVector<StringRef, 0> Names; - NameStrings.split(Names, ' '); + NameStrings.split(Names, getInstrProfNameSeparator()); for (StringRef &Name : Names) Symtab.addFuncName(Name); diff --git a/llvm/test/tools/llvm-cov/Inputs/binary-formats.v2.linux32l b/llvm/test/tools/llvm-cov/Inputs/binary-formats.v2.linux32l Binary files differindex 624483bf6e3..6ea83e5d24e 100755 --- a/llvm/test/tools/llvm-cov/Inputs/binary-formats.v2.linux32l +++ b/llvm/test/tools/llvm-cov/Inputs/binary-formats.v2.linux32l diff --git a/llvm/test/tools/llvm-cov/Inputs/binary-formats.v2.linux64l b/llvm/test/tools/llvm-cov/Inputs/binary-formats.v2.linux64l Binary files differindex af4d39cedfb..9c6835cdff1 100755 --- a/llvm/test/tools/llvm-cov/Inputs/binary-formats.v2.linux64l +++ b/llvm/test/tools/llvm-cov/Inputs/binary-formats.v2.linux64l diff --git a/llvm/test/tools/llvm-cov/Inputs/elf_binary_comdat.profdata b/llvm/test/tools/llvm-cov/Inputs/elf_binary_comdat.profdata Binary files differindex 85667041e2c..ef80e56a884 100644 --- a/llvm/test/tools/llvm-cov/Inputs/elf_binary_comdat.profdata +++ b/llvm/test/tools/llvm-cov/Inputs/elf_binary_comdat.profdata diff --git a/llvm/test/tools/llvm-cov/Inputs/instrprof-comdat.h b/llvm/test/tools/llvm-cov/Inputs/instrprof-comdat.h index e1227167cd4..a701504e34b 100644 --- a/llvm/test/tools/llvm-cov/Inputs/instrprof-comdat.h +++ b/llvm/test/tools/llvm-cov/Inputs/instrprof-comdat.h @@ -13,7 +13,12 @@ template <class T> T FOO<T>::DoIt(T ti) { // HEADER: 2| [[@LINE]]|template t += I; // HEADER: 20| [[@LINE]]| t += I; if (I > ti / 2) // HEADER: 20| [[@LINE]]| if (I > ti t -= 1; // HEADER: 8| [[@LINE]]| t -= 1; - } // HEADER: 10| [[@LINE]]| } - // HEADER: 1| [[@LINE]]| - return t; // HEADER: 1| [[@LINE]]| return t; + } // HEADER: 20| [[@LINE]]| } + // HEADER: 2| [[@LINE]]| + return t; // HEADER: 2| [[@LINE]]| return t; } + +// To generate the binaries which correspond to this file, you must first +// compile a program with two calls to Foo<int>::DoIt(10) for each desired +// architecture. Collect a raw profile from any one of these binaries, index +// it, and check it in along with the executables. diff --git a/llvm/test/tools/llvm-profdata/Inputs/c-general.profraw b/llvm/test/tools/llvm-profdata/Inputs/c-general.profraw Binary files differindex 602838369a2..34448c17b70 100644 --- a/llvm/test/tools/llvm-profdata/Inputs/c-general.profraw +++ b/llvm/test/tools/llvm-profdata/Inputs/c-general.profraw diff --git a/llvm/test/tools/llvm-profdata/raw-32-bits-be.test b/llvm/test/tools/llvm-profdata/raw-32-bits-be.test index d3def18ef99..bc6ec022587 100644 --- a/llvm/test/tools/llvm-profdata/raw-32-bits-be.test +++ b/llvm/test/tools/llvm-profdata/raw-32-bits-be.test @@ -28,7 +28,7 @@ RUN: printf '\0\0\0\0\0\0\0\0' >> %t RUN: printf '\0\0\0\0\0\0\0\023' >> %t RUN: printf '\0\0\0\0\0\0\0\067' >> %t RUN: printf '\0\0\0\0\0\0\0\101' >> %t -RUN: printf '\7\0foo bar\0\0\0\0\0\0\0' >> %t +RUN: printf '\7\0foo\1bar\0\0\0\0\0\0\0' >> %t RUN: llvm-profdata show %t -all-functions -counts | FileCheck %s diff --git a/llvm/test/tools/llvm-profdata/raw-32-bits-le.test b/llvm/test/tools/llvm-profdata/raw-32-bits-le.test index b19c74f01b6..5ee8b33df67 100644 --- a/llvm/test/tools/llvm-profdata/raw-32-bits-le.test +++ b/llvm/test/tools/llvm-profdata/raw-32-bits-le.test @@ -28,7 +28,7 @@ RUN: printf '\0\0\0\0\0\0\0\0' >> %t RUN: printf '\023\0\0\0\0\0\0\0' >> %t RUN: printf '\067\0\0\0\0\0\0\0' >> %t RUN: printf '\101\0\0\0\0\0\0\0' >> %t -RUN: printf '\7\0foo bar\0\0\0\0\0\0\0' >> %t +RUN: printf '\7\0foo\1bar\0\0\0\0\0\0\0' >> %t RUN: llvm-profdata show %t -all-functions -counts | FileCheck %s diff --git a/llvm/test/tools/llvm-profdata/raw-64-bits-be.test b/llvm/test/tools/llvm-profdata/raw-64-bits-be.test index cb59f9a2ff3..9778f062779 100644 --- a/llvm/test/tools/llvm-profdata/raw-64-bits-be.test +++ b/llvm/test/tools/llvm-profdata/raw-64-bits-be.test @@ -26,7 +26,7 @@ RUN: printf '\0\0\0\02\0\0\0\0' >> %t RUN: printf '\0\0\0\0\0\0\0\023' >> %t RUN: printf '\0\0\0\0\0\0\0\067' >> %t RUN: printf '\0\0\0\0\0\0\0\101' >> %t -RUN: printf '\7\0foo bar\0\0\0\0\0\0\0' >> %t +RUN: printf '\7\0foo\1bar\0\0\0\0\0\0\0' >> %t RUN: llvm-profdata show %t -all-functions -counts | FileCheck %s diff --git a/llvm/test/tools/llvm-profdata/raw-64-bits-le.test b/llvm/test/tools/llvm-profdata/raw-64-bits-le.test index 86748b14938..670bdb3ba85 100644 --- a/llvm/test/tools/llvm-profdata/raw-64-bits-le.test +++ b/llvm/test/tools/llvm-profdata/raw-64-bits-le.test @@ -26,7 +26,7 @@ RUN: printf '\02\0\0\0\0\0\0\0' >> %t RUN: printf '\023\0\0\0\0\0\0\0' >> %t RUN: printf '\067\0\0\0\0\0\0\0' >> %t RUN: printf '\101\0\0\0\0\0\0\0' >> %t -RUN: printf '\7\0foo bar\0\0\0\0\0\0\0' >> %t +RUN: printf '\7\0foo\1bar\0\0\0\0\0\0\0' >> %t RUN: llvm-profdata show %t -all-functions -counts | FileCheck %s diff --git a/llvm/unittests/ProfileData/InstrProfTest.cpp b/llvm/unittests/ProfileData/InstrProfTest.cpp index 13d1e059cd2..7dd072c849b 100644 --- a/llvm/unittests/ProfileData/InstrProfTest.cpp +++ b/llvm/unittests/ProfileData/InstrProfTest.cpp @@ -893,7 +893,7 @@ TEST_P(MaybeSparseInstrProfTest, instr_prof_symtab_compression_test) { OS << "func_" << I; FuncNames1.push_back(OS.str()); str.clear(); - OS << "fooooooooooooooo_" << I; + OS << "f oooooooooooooo_" << I; FuncNames1.push_back(OS.str()); str.clear(); OS << "BAR_" << I; @@ -931,7 +931,7 @@ TEST_P(MaybeSparseInstrProfTest, instr_prof_symtab_compression_test) { StringRef R = Symtab.getFuncName(IndexedInstrProf::ComputeHash(FuncNames1[0])); ASSERT_EQ(StringRef("func_0"), R); R = Symtab.getFuncName(IndexedInstrProf::ComputeHash(FuncNames1[1])); - ASSERT_EQ(StringRef("fooooooooooooooo_0"), R); + ASSERT_EQ(StringRef("f oooooooooooooo_0"), R); for (int I = 0; I < 3; I++) { std::string N[4]; N[0] = FuncNames1[2 * I]; |