From ca3fdca1013e98c18efb8f57feb2ba5d33ab8f5f Mon Sep 17 00:00:00 2001 From: Justin Bogner Date: Tue, 17 Feb 2015 09:21:43 +0000 Subject: [PATCH] Revert "InstrProf: Add unit tests for the profile reader and writer" This added API to the InstrProfWriter to write to a string so I could write unittests without using temp files. This doesn't really work, since the format has tighter alignment requirements than a char. This reverts r229478 and its follow-up, r229481. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@229483 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/ProfileData/InstrProfReader.h | 6 -- include/llvm/ProfileData/InstrProfWriter.h | 6 +- lib/ProfileData/InstrProfReader.cpp | 29 +++---- lib/ProfileData/InstrProfWriter.cpp | 30 +------ unittests/ProfileData/CMakeLists.txt | 1 - unittests/ProfileData/InstrProfTest.cpp | 97 ---------------------- 6 files changed, 15 insertions(+), 154 deletions(-) delete mode 100644 unittests/ProfileData/InstrProfTest.cpp diff --git a/include/llvm/ProfileData/InstrProfReader.h b/include/llvm/ProfileData/InstrProfReader.h index 63a6ac671f2..5dd1d4f9dd7 100644 --- a/include/llvm/ProfileData/InstrProfReader.h +++ b/include/llvm/ProfileData/InstrProfReader.h @@ -95,9 +95,6 @@ public: /// Factory method to create an appropriately typed reader for the given /// instrprof file. static ErrorOr> create(std::string Path); - - static ErrorOr> - create(std::unique_ptr Buffer); }; /// Reader for the simple text based instrprof format. @@ -297,9 +294,6 @@ public: /// Factory method to create an indexed reader. static ErrorOr> create(std::string Path); - - static ErrorOr> - create(std::unique_ptr Buffer); }; } // end namespace llvm diff --git a/include/llvm/ProfileData/InstrProfWriter.h b/include/llvm/ProfileData/InstrProfWriter.h index 48836e15124..a23c56772a2 100644 --- a/include/llvm/ProfileData/InstrProfWriter.h +++ b/include/llvm/ProfileData/InstrProfWriter.h @@ -41,12 +41,8 @@ public: std::error_code addFunctionCounts(StringRef FunctionName, uint64_t FunctionHash, ArrayRef Counters); - /// Write the profile to \c OS + /// Ensure that all data is written to disk. void write(raw_fd_ostream &OS); - /// Write the profile, returning the raw data. For testing. - std::string writeString(); -private: - std::pair writeImpl(raw_ostream &OS); }; } // end namespace llvm diff --git a/lib/ProfileData/InstrProfReader.cpp b/lib/ProfileData/InstrProfReader.cpp index 01e199dcf0e..d13f27c3113 100644 --- a/lib/ProfileData/InstrProfReader.cpp +++ b/lib/ProfileData/InstrProfReader.cpp @@ -25,7 +25,12 @@ setupMemoryBuffer(std::string Path) { MemoryBuffer::getFileOrSTDIN(Path); if (std::error_code EC = BufferOrErr.getError()) return EC; - return std::move(BufferOrErr.get()); + auto Buffer = std::move(BufferOrErr.get()); + + // Sanity check the file. + if (Buffer->getBufferSize() > std::numeric_limits::max()) + return instrprof_error::too_large; + return std::move(Buffer); } static std::error_code initializeReader(InstrProfReader &Reader) { @@ -38,16 +43,10 @@ InstrProfReader::create(std::string Path) { auto BufferOrError = setupMemoryBuffer(Path); if (std::error_code EC = BufferOrError.getError()) return EC; - return InstrProfReader::create(std::move(BufferOrError.get())); -} - -ErrorOr> -InstrProfReader::create(std::unique_ptr Buffer) { - // Sanity check the buffer. - if (Buffer->getBufferSize() > std::numeric_limits::max()) - return instrprof_error::too_large; + auto Buffer = std::move(BufferOrError.get()); std::unique_ptr Result; + // Create the reader. if (IndexedInstrProfReader::hasFormat(*Buffer)) Result.reset(new IndexedInstrProfReader(std::move(Buffer))); @@ -71,20 +70,14 @@ IndexedInstrProfReader::create(std::string Path) { auto BufferOrError = setupMemoryBuffer(Path); if (std::error_code EC = BufferOrError.getError()) return EC; - return IndexedInstrProfReader::create(std::move(BufferOrError.get())); -} - -ErrorOr> -IndexedInstrProfReader::create(std::unique_ptr Buffer) { - // Sanity check the buffer. - if (Buffer->getBufferSize() > std::numeric_limits::max()) - return instrprof_error::too_large; + auto Buffer = std::move(BufferOrError.get()); + std::unique_ptr Result; // Create the reader. if (!IndexedInstrProfReader::hasFormat(*Buffer)) return instrprof_error::bad_magic; - auto Result = llvm::make_unique(std::move(Buffer)); + Result.reset(new IndexedInstrProfReader(std::move(Buffer))); // Initialize the reader and return the result. if (std::error_code EC = initializeReader(*Result)) diff --git a/lib/ProfileData/InstrProfWriter.cpp b/lib/ProfileData/InstrProfWriter.cpp index 2c72efe3faa..d4cde2e195d 100644 --- a/lib/ProfileData/InstrProfWriter.cpp +++ b/lib/ProfileData/InstrProfWriter.cpp @@ -106,7 +106,7 @@ InstrProfWriter::addFunctionCounts(StringRef FunctionName, return instrprof_error::success; } -std::pair InstrProfWriter::writeImpl(raw_ostream &OS) { +void InstrProfWriter::write(raw_fd_ostream &OS) { OnDiskChainedHashTableGenerator Generator; // Populate the hash table generator. @@ -128,31 +128,7 @@ std::pair InstrProfWriter::writeImpl(raw_ostream &OS) { // Write the hash table. uint64_t HashTableStart = Generator.Emit(OS); - return std::make_pair(HashTableStartLoc, HashTableStart); -} - -void InstrProfWriter::write(raw_fd_ostream &OS) { - // Write the hash table. - auto TableStart = writeImpl(OS); - // Go back and fill in the hash table start. - using namespace support; - OS.seek(TableStart.first); - endian::Writer(OS).write(TableStart.second); -} - -std::string InstrProfWriter::writeString() { - std::string Result; - llvm::raw_string_ostream OS(Result); - // Write the hash table. - auto TableStart = writeImpl(OS); - OS.flush(); - - // Go back and fill in the hash table start. - using namespace support; - uint64_t Bytes = endian::byte_swap(TableStart.second); - Result.replace(TableStart.first, sizeof(uint64_t), (const char *)&Bytes, - sizeof(uint64_t)); - - return Result; + OS.seek(HashTableStartLoc); + LE.write(HashTableStart); } diff --git a/unittests/ProfileData/CMakeLists.txt b/unittests/ProfileData/CMakeLists.txt index 79137c9510a..3251ff41502 100644 --- a/unittests/ProfileData/CMakeLists.txt +++ b/unittests/ProfileData/CMakeLists.txt @@ -6,5 +6,4 @@ set(LLVM_LINK_COMPONENTS add_llvm_unittest(ProfileDataTests CoverageMappingTest.cpp - InstrProfTest.cpp ) diff --git a/unittests/ProfileData/InstrProfTest.cpp b/unittests/ProfileData/InstrProfTest.cpp deleted file mode 100644 index 1a25a1fd90d..00000000000 --- a/unittests/ProfileData/InstrProfTest.cpp +++ /dev/null @@ -1,97 +0,0 @@ -//===- unittest/ProfileData/InstrProfTest.cpp -------------------------------=// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===----------------------------------------------------------------------===// - -#include "llvm/ProfileData/InstrProfReader.h" -#include "llvm/ProfileData/InstrProfWriter.h" -#include "gtest/gtest.h" - -#include - -using namespace llvm; - -namespace { - -struct InstrProfTest : ::testing::Test { - InstrProfWriter Writer; - std::unique_ptr Reader; - - void addCounts(StringRef Name, uint64_t Hash, int NumCounts, ...) { - SmallVector Counts; - va_list Args; - va_start(Args, NumCounts); - for (int I = 0; I < NumCounts; ++I) - Counts.push_back(va_arg(Args, uint64_t)); - va_end(Args); - Writer.addFunctionCounts(Name, Hash, Counts); - } - - std::string writeProfile() { return Writer.writeString(); } - void readProfile(std::string Profile) { - auto ReaderOrErr = - IndexedInstrProfReader::create(MemoryBuffer::getMemBuffer(Profile)); - ASSERT_EQ(std::error_code(), ReaderOrErr.getError()); - Reader = std::move(ReaderOrErr.get()); - } -}; - -TEST_F(InstrProfTest, write_and_read_empty_profile) { - std::string Profile = writeProfile(); - readProfile(Profile); - ASSERT_TRUE(Reader->begin() == Reader->end()); -} - -TEST_F(InstrProfTest, write_and_read_one_function) { - addCounts("foo", 0x1234, 4, 1ULL, 2ULL, 3ULL, 4ULL); - std::string Profile = writeProfile(); - readProfile(Profile); - - auto I = Reader->begin(), E = Reader->end(); - ASSERT_TRUE(I != E); - ASSERT_EQ(StringRef("foo"), I->Name); - ASSERT_EQ(0x1234U, I->Hash); - ASSERT_EQ(4U, I->Counts.size()); - ASSERT_EQ(1U, I->Counts[0]); - ASSERT_EQ(2U, I->Counts[1]); - ASSERT_EQ(3U, I->Counts[2]); - ASSERT_EQ(4U, I->Counts[3]); - ASSERT_TRUE(++I == E); -} - -TEST_F(InstrProfTest, get_function_counts) { - addCounts("foo", 0x1234, 2, 1ULL, 2ULL); - std::string Profile = writeProfile(); - readProfile(Profile); - - std::vector Counts; - std::error_code EC; - - EC = Reader->getFunctionCounts("foo", 0x1234, Counts); - ASSERT_EQ(instrprof_error::success, EC); - ASSERT_EQ(2U, Counts.size()); - ASSERT_EQ(1U, Counts[0]); - ASSERT_EQ(2U, Counts[1]); - - EC = Reader->getFunctionCounts("foo", 0x5678, Counts); - ASSERT_EQ(instrprof_error::hash_mismatch, EC); - - EC = Reader->getFunctionCounts("bar", 0x1234, Counts); - ASSERT_EQ(instrprof_error::unknown_function, EC); -} - -TEST_F(InstrProfTest, get_max_function_count) { - addCounts("foo", 0x1234, 2, 1ULL << 31, 2ULL); - addCounts("bar", 0, 1, 1ULL << 63); - addCounts("baz", 0x5678, 4, 0ULL, 0ULL, 0ULL, 0ULL); - std::string Profile = writeProfile(); - readProfile(Profile); - - ASSERT_EQ(1ULL << 63, Reader->getMaximumFunctionCount()); -} - -} // end anonymous namespace -- 2.34.1