Author: Mingming Liu Date: 2023-11-22T21:22:20-08:00 New Revision: 9ab133bd9f9cedb880b453dd830b58857fab41ec
URL: https://github.com/llvm/llvm-project/commit/9ab133bd9f9cedb880b453dd830b58857fab41ec DIFF: https://github.com/llvm/llvm-project/commit/9ab133bd9f9cedb880b453dd830b58857fab41ec.diff LOG: [nfc][InstrProfTest]Parameterize the edge cases of value profile merge by value kind (#73165) There are three test cases to test the merge of value profiles. 'get_icall_data_merge1' tests the basic case; {get_icall_data_merge1_saturation, get_icall_data_merge_site_trunc} tests the edge case. This patch parameterizes the edge case test coverage by value kind and adds the coverage of 'IPVK_MemOPSize'. Keep the basic test structure as it is. The main reason is test data construction and test assertions is clearer for each kind in the basic test. - Using a loop for different value kinds in one test case doesn't work very well. The instr-prof-writer is stateful (e.g., keeps track of per-function profile data in a [container](https://github.com/llvm/llvm-project/blob/a9c149df7666bb2f8755794b97573134e5cfeb38/llvm/include/llvm/ProfileData/InstrProfWriter.h#L43)) Added: Modified: llvm/unittests/ProfileData/InstrProfTest.cpp Removed: ################################################################################ diff --git a/llvm/unittests/ProfileData/InstrProfTest.cpp b/llvm/unittests/ProfileData/InstrProfTest.cpp index 875e2d06d839367..e6613a90dc7c53e 100644 --- a/llvm/unittests/ProfileData/InstrProfTest.cpp +++ b/llvm/unittests/ProfileData/InstrProfTest.cpp @@ -815,7 +815,7 @@ TEST_P(MaybeSparseInstrProfTest, annotate_vp_data) { ASSERT_EQ(1U, ValueData[3].Count); } -TEST_P(MaybeSparseInstrProfTest, get_icall_data_merge1) { +TEST_P(MaybeSparseInstrProfTest, icall_data_merge) { static const char caller[] = "caller"; NamedInstrProfRecord Record11(caller, 0x1234, {1, 2}); NamedInstrProfRecord Record12(caller, 0x1234, {1, 2}); @@ -920,8 +920,18 @@ TEST_P(MaybeSparseInstrProfTest, get_icall_data_merge1) { ASSERT_EQ(2U, VD_4[2].Count); } -TEST_P(MaybeSparseInstrProfTest, get_icall_data_merge1_saturation) { +struct ValueProfileMergeEdgeCaseTest + : public InstrProfTest, + public ::testing::WithParamInterface<std::tuple<bool, uint32_t>> { + void SetUp() override { Writer.setOutputSparse(std::get<0>(GetParam())); } + + uint32_t getValueProfileKind() const { return std::get<1>(GetParam()); } +}; + +TEST_P(ValueProfileMergeEdgeCaseTest, value_profile_data_merge_saturation) { + const uint32_t ValueKind = getValueProfileKind(); static const char bar[] = "bar"; + const uint64_t ProfiledValue = 0x5678; const uint64_t MaxValCount = std::numeric_limits<uint64_t>::max(); const uint64_t MaxEdgeCount = getInstrMaxCountValue(); @@ -944,18 +954,18 @@ TEST_P(MaybeSparseInstrProfTest, get_icall_data_merge1_saturation) { ASSERT_EQ(Result, instrprof_error::success); NamedInstrProfRecord Record4("baz", 0x5678, {3, 4}); - Record4.reserveSites(IPVK_IndirectCallTarget, 1); - InstrProfValueData VD4[] = {{uint64_t(bar), 1}}; - Record4.addValueData(IPVK_IndirectCallTarget, 0, VD4, 1, nullptr); + Record4.reserveSites(ValueKind, 1); + InstrProfValueData VD4[] = {{ProfiledValue, 1}}; + Record4.addValueData(ValueKind, 0, VD4, 1, nullptr); Result = instrprof_error::success; Writer.addRecord(std::move(Record4), Err); ASSERT_EQ(Result, instrprof_error::success); // Verify value data counter overflow. NamedInstrProfRecord Record5("baz", 0x5678, {5, 6}); - Record5.reserveSites(IPVK_IndirectCallTarget, 1); - InstrProfValueData VD5[] = {{uint64_t(bar), MaxValCount}}; - Record5.addValueData(IPVK_IndirectCallTarget, 0, VD5, 1, nullptr); + Record5.reserveSites(ValueKind, 1); + InstrProfValueData VD5[] = {{ProfiledValue, MaxValCount}}; + Record5.addValueData(ValueKind, 0, VD5, 1, nullptr); Result = instrprof_error::success; Writer.addRecord(std::move(Record5), Err); ASSERT_EQ(Result, instrprof_error::counter_overflow); @@ -966,48 +976,48 @@ TEST_P(MaybeSparseInstrProfTest, get_icall_data_merge1_saturation) { // Verify saturation of counts. Expected<InstrProfRecord> ReadRecord1 = Reader->getInstrProfRecord("foo", 0x1234); - EXPECT_THAT_ERROR(ReadRecord1.takeError(), Succeeded()); - ASSERT_EQ(MaxEdgeCount, ReadRecord1->Counts[0]); + ASSERT_THAT_ERROR(ReadRecord1.takeError(), Succeeded()); + EXPECT_EQ(MaxEdgeCount, ReadRecord1->Counts[0]); Expected<InstrProfRecord> ReadRecord2 = Reader->getInstrProfRecord("baz", 0x5678); ASSERT_TRUE(bool(ReadRecord2)); - ASSERT_EQ(1U, ReadRecord2->getNumValueSites(IPVK_IndirectCallTarget)); + ASSERT_EQ(1U, ReadRecord2->getNumValueSites(ValueKind)); std::unique_ptr<InstrProfValueData[]> VD = - ReadRecord2->getValueForSite(IPVK_IndirectCallTarget, 0); - ASSERT_EQ(StringRef("bar"), StringRef((const char *)VD[0].Value, 3)); - ASSERT_EQ(MaxValCount, VD[0].Count); + ReadRecord2->getValueForSite(ValueKind, 0); + EXPECT_EQ(ProfiledValue, VD[0].Value); + EXPECT_EQ(MaxValCount, VD[0].Count); } -// This test tests that when there are too many values -// for a given site, the merged results are properly -// truncated. -TEST_P(MaybeSparseInstrProfTest, get_icall_data_merge_site_trunc) { +// This test tests that when there are too many values for a given site, the +// merged results are properly truncated. +TEST_P(ValueProfileMergeEdgeCaseTest, value_profile_data_merge_site_trunc) { + const uint32_t ValueKind = getValueProfileKind(); static const char caller[] = "caller"; NamedInstrProfRecord Record11(caller, 0x1234, {1, 2}); NamedInstrProfRecord Record12(caller, 0x1234, {1, 2}); // 2 value sites. - Record11.reserveSites(IPVK_IndirectCallTarget, 2); + Record11.reserveSites(ValueKind, 2); InstrProfValueData VD0[255]; for (int I = 0; I < 255; I++) { VD0[I].Value = 2 * I; VD0[I].Count = 2 * I + 1000; } - Record11.addValueData(IPVK_IndirectCallTarget, 0, VD0, 255, nullptr); - Record11.addValueData(IPVK_IndirectCallTarget, 1, nullptr, 0, nullptr); + Record11.addValueData(ValueKind, 0, VD0, 255, nullptr); + Record11.addValueData(ValueKind, 1, nullptr, 0, nullptr); - Record12.reserveSites(IPVK_IndirectCallTarget, 2); + Record12.reserveSites(ValueKind, 2); InstrProfValueData VD1[255]; for (int I = 0; I < 255; I++) { VD1[I].Value = 2 * I + 1; VD1[I].Count = 2 * I + 1001; } - Record12.addValueData(IPVK_IndirectCallTarget, 0, VD1, 255, nullptr); - Record12.addValueData(IPVK_IndirectCallTarget, 1, nullptr, 0, nullptr); + Record12.addValueData(ValueKind, 0, VD1, 255, nullptr); + Record12.addValueData(ValueKind, 1, nullptr, 0, nullptr); Writer.addRecord(std::move(Record11), Err); // Merge profile data. @@ -1017,17 +1027,23 @@ TEST_P(MaybeSparseInstrProfTest, get_icall_data_merge_site_trunc) { readProfile(std::move(Profile)); Expected<InstrProfRecord> R = Reader->getInstrProfRecord("caller", 0x1234); - EXPECT_THAT_ERROR(R.takeError(), Succeeded()); - std::unique_ptr<InstrProfValueData[]> VD( - R->getValueForSite(IPVK_IndirectCallTarget, 0)); - ASSERT_EQ(2U, R->getNumValueSites(IPVK_IndirectCallTarget)); - ASSERT_EQ(255U, R->getNumValueDataForSite(IPVK_IndirectCallTarget, 0)); + ASSERT_THAT_ERROR(R.takeError(), Succeeded()); + std::unique_ptr<InstrProfValueData[]> VD(R->getValueForSite(ValueKind, 0)); + ASSERT_EQ(2U, R->getNumValueSites(ValueKind)); + EXPECT_EQ(255U, R->getNumValueDataForSite(ValueKind, 0)); for (unsigned I = 0; I < 255; I++) { - ASSERT_EQ(VD[I].Value, 509 - I); - ASSERT_EQ(VD[I].Count, 1509 - I); + EXPECT_EQ(VD[I].Value, 509 - I); + EXPECT_EQ(VD[I].Count, 1509 - I); } } +INSTANTIATE_TEST_SUITE_P( + EdgeCaseTest, ValueProfileMergeEdgeCaseTest, + ::testing::Combine(::testing::Bool(), /* Sparse */ + ::testing::Values(IPVK_IndirectCallTarget, + IPVK_MemOPSize) /* ValueKind */ + )); + static void addValueProfData(InstrProfRecord &Record) { Record.reserveSites(IPVK_IndirectCallTarget, 5); InstrProfValueData VD0[] = {{uint64_t(callee1), 400}, _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
