[PATCH] D18568: [PGO] Use ArrayRef in annotateValueSite()
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 29 22:31:44 PDT 2016
On Tue, Mar 29, 2016 at 12:49 PM, Rong Xu via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> xur created this revision.
> xur added reviewers: silvas, davidxl.
> xur added subscribers: xur, llvm-commits.
>
> 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.
>
> 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,9 @@
> // 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);
> + ArrayRef<InstrProfValueData> VDArrayRef(VD0Sorted, 6);
>
Can you just use makeArrayRef here ^ (so you don't have to duplicate the
array length)? (or at least use array_lengthof rather than hardcoding it)
> + annotateValueSite(*M, *Inst, VDArrayRef.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,
>
Const on a value parameter is pretty non-idiomatic. Perhaps drop the const?
> 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 &I : VDArrayRef) {
> Vals.push_back(MDHelper.createConstant(
> - ConstantInt::get(Type::getInt64Ty(Ctx), VD[I].Value)));
> + ConstantInt::get(Type::getInt64Ty(Ctx), I.Value)));
> Vals.push_back(MDHelper.createConstant(
> - ConstantInt::get(Type::getInt64Ty(Ctx), VD[I].Count)));
> + ConstantInt::get(Type::getInt64Ty(Ctx), I.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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160329/ab0127d2/attachment.html>
More information about the llvm-commits
mailing list