[llvm] [nfc][InstrProfTest]Factor out common code for value profile test (PR #72611)

Mingming Liu via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 21 12:45:24 PST 2023


================
@@ -64,13 +64,130 @@ struct InstrProfTest : ::testing::Test {
   }
 };
 
+static const char callee1[] = "callee1";
+static const char callee2[] = "callee2";
+static const char callee3[] = "callee3";
+static const char callee4[] = "callee4";
+static const char callee5[] = "callee5";
+static const char callee6[] = "callee6";
+
+static const auto Err = [](Error E) {
+  consumeError(std::move(E));
+  FAIL();
+};
+
+typedef std::vector<MutableArrayRef<InstrProfValueData>> VDArray;
+
+// This helper function adds the value profile data to Record. The type of
+// value profiles is specified by 'ValueKind'. 'ValueDataArray' is a non-const
+// reference and the vector element is a mutable array reference. This is mainly
+// because method `InstrProfRecord::addValueData` takes a pointer and might
+// modify the pointed-to content.
+static void addValueProfData(InstrProfRecord &Record, uint32_t ValueKind,
+                             VDArray &ValueDataArray) {
+  Record.reserveSites(ValueKind, ValueDataArray.size());
+  for (long unsigned int i = 0; i < ValueDataArray.size(); i++) {
+    // The state of vector::data() is not specified when the vector is empty,
+    // and MutableArrayRef takes vector::data() when initialized with a vector.
+    Record.addValueData(ValueKind, i,
+                        ValueDataArray[i].empty() ? nullptr
+                                                  : ValueDataArray[i].data(),
+                        ValueDataArray[i].size(), nullptr);
+  }
+}
+
 struct SparseInstrProfTest : public InstrProfTest {
   void SetUp() override { Writer.setOutputSparse(true); }
 };
 
 struct MaybeSparseInstrProfTest : public InstrProfTest,
                                   public ::testing::WithParamInterface<bool> {
   void SetUp() override { Writer.setOutputSparse(GetParam()); }
+
+public:
+  // Tests that value profiles in Record has the same content as (possibly
+  // weighted and sorted) InputVDs for each value kind. ValueProfSorted is true
+  // iff the value profiles of Record are already sorted by count.
+  // InputVDs is a non-const reference since it might need a in-place sort.
+  void testValueDataArray(
+      std::vector<std::pair<uint32_t /* ValueKind */, VDArray &>> &InputVDs,
+      InstrProfRecord &Record, bool ValueProfSorted = false,
+      uint64_t ProfWeight = 1) {
+    for (auto &[ValueKind, InputVD] : InputVDs) {
+      ASSERT_EQ(InputVD.size(), Record.getNumValueSites(ValueKind));
+      for (unsigned i = 0; i < InputVD.size(); i++) {
+        ASSERT_EQ(InputVD[i].size(),
+                  Record.getNumValueDataForSite(ValueKind, i));
+
+        uint64_t WantTotalC = 0, GetTotalC = 0;
+        std::unique_ptr<InstrProfValueData[]> OutputVD =
+            Record.getValueForSite(ValueKind, i, &GetTotalC);
+
+        // If value profile elements of the same instrumented site are sorted by
+        // count in Record, sort the input value data array the same way for
+        // comparison purpose.
+        if (ValueProfSorted) {
+          llvm::stable_sort(InputVD[i], [](const InstrProfValueData &lhs,
+                                           const InstrProfValueData &rhs) {
+            return lhs.Count > rhs.Count;
+          });
+        }
+
+        // The previous ASSERT_EQ already tests the number of value data is
+        // InputVD[i].size(), so there won't be out-of-bound access.
+        for (unsigned j = 0; j < InputVD[i].size(); j++) {
+          EXPECT_EQ(InputVD[i][j].Count * ProfWeight, OutputVD[j].Count);
+          EXPECT_EQ(InputVD[i][j].Value, OutputVD[j].Value);
+          WantTotalC += InputVD[i][j].Count;
+        }
+        EXPECT_EQ(WantTotalC * ProfWeight, GetTotalC);
+      }
+    }
+  }
+
+  // A helper function to test the writes and reads of indirect call value
+  // profiles. The profile writer will scale counters by `ProfWeight` when
+  // adding a record. `Endianness` specifies the endianness used by profile
+  // writer and reader when handling value profile records.
+  void testICallDataReadWrite(
+      uint64_t ProfWeight = 1,
+      llvm::endianness Endianness = llvm::endianness::little) {
----------------
minglotus-6 wrote:

As chatted offline, I realized test coverage for a new kind of value-type might double the amount of code without factoring out common code once I attempted to add new tests. Thanks for going over the test cases together and discussing about how to properly clean it up and prepare for the new value type. I'm sending out smaller patches.

https://github.com/llvm/llvm-project/pull/72611


More information about the llvm-commits mailing list