[PGO] Use ArrayRef in annotateValueSite()

[xur]

Summary

Using ArrayRef in annotateValueSite's parameter instead of using an array and it's size. This is one of cleanups suggested by Sean Silva in http://reviews.llvm.org/D17864.

[silvas]

Thank! LGTM. A couple minor suggestions inline.

lib/ProfileData/InstrProf.cpp

631

Would VD be a better name instead of I?

unittests/ProfileData/InstrProfTest.cpp

334

I believe this can be just makeArrayRef(VD0Sorted).slice(2) without introducing the temporary or having to explicitly give a size.

[xur]

Sean, thanks for the quick review. Here is the updated patch.

Regards,

-Rong

[xur]

Justin, thanks for the suggestion. Attached is the updated patch.

[silvas]

LGTM

include/llvm/ProfileData/InstrProf.h

 
 /// Get the value profile data for value site \p SiteIdx from \p InstrProfR
 /// and annotate the instruction \p Inst with the value profile meta data.
 /// Annotate up to \p MaxMDCount (default 3) number of records per value site.
 void annotateValueSite(Module &M, Instruction &Inst,
                        const InstrProfRecord &InstrProfR,
                        InstrProfValueKind ValueKind, uint32_t SiteIndx,
                        uint32_t MaxMDCount = 3);
-/// Same as the above interface but using the ValueData array directly, as
-/// well as \p Sum.
+/// Same as the above interface but using an ArrayRef, as well as \p Sum.
 void annotateValueSite(Module &M, Instruction &Inst,
-                       const InstrProfValueData VD[], uint32_t NV,
+                       const ArrayRef<InstrProfValueData> VDArrayRef,
                        uint64_t Sum, InstrProfValueKind ValueKind,
                        uint32_t MaxMDCount);
 
 /// Extract the value profile data from \p Inst which is annotated with
 /// value profile meta data. Return false if there is no value data annotated,
 /// otherwise  return true.
 bool getValueProfDataFromInst(const Instruction &Inst,
                               InstrProfValueKind ValueKind,

lib/ProfileData/InstrProf.cpp

void annotateValueSite(Module &M, Instruction &Inst,
                        InstrProfValueKind ValueKind, uint32_t SiteIdx,
                        uint32_t MaxMDCount) {
   uint32_t NV = InstrProfR.getNumValueDataForSite(ValueKind, SiteIdx);
 
   uint64_t Sum = 0;
   std::unique_ptr<InstrProfValueData[]> VD =
       InstrProfR.getValueForSite(ValueKind, SiteIdx, &Sum);
 
-  annotateValueSite(M, Inst, VD.get(), NV, Sum, ValueKind, MaxMDCount);
+  ArrayRef<InstrProfValueData> VDArrayRef(VD.get(), NV);
+  annotateValueSite(M, Inst, VDArrayRef, Sum, ValueKind, MaxMDCount);
 }
 
 void annotateValueSite(Module &M, Instruction &Inst,
-                       const InstrProfValueData VD[], uint32_t NV,
+                       const ArrayRef<InstrProfValueData> VDArrayRef,
                        uint64_t Sum, InstrProfValueKind ValueKind,
                        uint32_t MaxMDCount) {
   LLVMContext &Ctx = M.getContext();
   MDBuilder MDHelper(Ctx);
   SmallVector<Metadata *, 3> Vals;
   // Tag
   Vals.push_back(MDHelper.createString("VP"));
   // Value Kind
   Vals.push_back(MDHelper.createConstant(
       ConstantInt::get(Type::getInt32Ty(Ctx), ValueKind)));
   // Total Count
   Vals.push_back(
       MDHelper.createConstant(ConstantInt::get(Type::getInt64Ty(Ctx), Sum)));
 
   // Value Profile Data
   uint32_t MDCount = MaxMDCount;
-  for (uint32_t I = 0; I < NV; ++I) {
+  for (auto &VD : VDArrayRef) {
     Vals.push_back(MDHelper.createConstant(
-        ConstantInt::get(Type::getInt64Ty(Ctx), VD[I].Value)));
+        ConstantInt::get(Type::getInt64Ty(Ctx), VD.Value)));
     Vals.push_back(MDHelper.createConstant(
-        ConstantInt::get(Type::getInt64Ty(Ctx), VD[I].Count)));
+        ConstantInt::get(Type::getInt64Ty(Ctx), VD.Count)));
     if (--MDCount == 0)
       break;
   }
   Inst.setMetadata(LLVMContext::MD_prof, MDNode::get(Ctx, Vals));
 }
 
 bool getValueProfDataFromInst(const Instruction &Inst,
                               InstrProfValueKind ValueKind,

unittests/ProfileData/InstrProfTest.cpp

TEST_P(MaybeSparseInstrProfTest, annotate_vp_data) {
   ASSERT_EQ(2000U, ValueData[4].Value);
   ASSERT_EQ(2U, ValueData[4].Count);
 
   // Remove the MD_prof metadata 
   Inst->setMetadata(LLVMContext::MD_prof, 0);
   // Annotate with 4 records.
   InstrProfValueData VD0Sorted[] = {{1000, 6}, {2000, 5}, {3000, 4}, {4000, 3},
                               {5000, 2}, {6000, 1}};
-  annotateValueSite(*M, *Inst, &VD0Sorted[2], 4, 10, IPVK_IndirectCallTarget, 5);
+  annotateValueSite(*M, *Inst, makeArrayRef(VD0Sorted).slice(2), 10,
+                    IPVK_IndirectCallTarget, 5);
   Res = getValueProfDataFromInst(*Inst, IPVK_IndirectCallTarget, 5,
                                       ValueData, N, T);
   ASSERT_TRUE(Res);
   ASSERT_EQ(4U, N);
   ASSERT_EQ(10U, T);
   ASSERT_EQ(3000U, ValueData[0].Value);
   ASSERT_EQ(4U, ValueData[0].Count);
   ASSERT_EQ(4000U, ValueData[1].Value);