[PATCH] D18568: [PGO] Use ArrayRef in annotateValueSite()

[Justin Bogner via llvm-commits]

Rong Xu via llvm-commits <llvm-commits at lists.llvm.org> writes:
> xur updated this revision to Diff 51971.
> xur added a comment.
>
> Sean, thanks for the quick review. Here is the updated patch.
>
> Regards,
>
> -Rong
>
>
> http://reviews.llvm.org/D18568
>
> Files:
>   include/llvm/ProfileData/InstrProf.h
>   lib/ProfileData/InstrProf.cpp
>   unittests/ProfileData/InstrProfTest.cpp
>
> Index: unittests/ProfileData/InstrProfTest.cpp
> ===================================================================
> --- unittests/ProfileData/InstrProfTest.cpp
> +++ unittests/ProfileData/InstrProfTest.cpp
> @@ -330,7 +330,8 @@
>    // 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);
> Index: lib/ProfileData/InstrProf.cpp
> ===================================================================
> --- lib/ProfileData/InstrProf.cpp
> +++ lib/ProfileData/InstrProf.cpp
> @@ -606,11 +606,12 @@
>    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,

VDArrayRef isn't a great name - nobody cares that it's an ArrayRef. I'd
probably just call it VDs or VDList or something. Either way, LGTM.

>                         uint64_t Sum, InstrProfValueKind ValueKind,
>                         uint32_t MaxMDCount) {
>    LLVMContext &Ctx = M.getContext();
> @@ -627,11 +628,11 @@
>  
>    // 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;
>    }
> Index: include/llvm/ProfileData/InstrProf.h
> ===================================================================
> --- include/llvm/ProfileData/InstrProf.h
> +++ include/llvm/ProfileData/InstrProf.h
> @@ -229,10 +229,9 @@
>                         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);
>  
>
>
> Index: unittests/ProfileData/InstrProfTest.cpp
> ===================================================================
> --- unittests/ProfileData/InstrProfTest.cpp
> +++ unittests/ProfileData/InstrProfTest.cpp
> @@ -330,7 +330,8 @@
>    // 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);
> Index: lib/ProfileData/InstrProf.cpp
> ===================================================================
> --- lib/ProfileData/InstrProf.cpp
> +++ lib/ProfileData/InstrProf.cpp
> @@ -606,11 +606,12 @@
>    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();
> @@ -627,11 +628,11 @@
>  
>    // 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;
>    }
> Index: include/llvm/ProfileData/InstrProf.h
> ===================================================================
> --- include/llvm/ProfileData/InstrProf.h
> +++ include/llvm/ProfileData/InstrProf.h
> @@ -229,10 +229,9 @@
>                         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);
>  
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits