<div dir="ltr">It can be -- but it is probably ok to keep it there in case it is used by common code in the future.<div><br></div><div>David</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, May 12, 2016 at 6:22 PM, Vedant Kumar <span dir="ltr"><<a href="mailto:vsk@apple.com" target="_blank">vsk@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Does this mean we can get rid of INSTR_PROF_NULLPTR now?<br>
<br>
vedant<br>
<br>
<br>
> On May 12, 2016, at 5:23 PM, Xinliang David Li via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
><br>
> Author: davidxl<br>
> Date: Thu May 12 19:23:49 2016<br>
> New Revision: 269384<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=269384&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=269384&view=rev</a><br>
> Log:<br>
> Remove runtime specific code from common header<br>
><br>
> Modified:<br>
> llvm/trunk/include/llvm/ProfileData/InstrProfData.inc<br>
> llvm/trunk/unittests/ProfileData/InstrProfTest.cpp<br>
><br>
> Modified: llvm/trunk/include/llvm/ProfileData/InstrProfData.inc<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ProfileData/InstrProfData.inc?rev=269384&r1=269383&r2=269384&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ProfileData/InstrProfData.inc?rev=269384&r1=269383&r2=269384&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/include/llvm/ProfileData/InstrProfData.inc (original)<br>
> +++ llvm/trunk/include/llvm/ProfileData/InstrProfData.inc Thu May 12 19:23:49 2016<br>
> @@ -384,16 +384,6 @@ typedef struct ValueProfRuntimeRecord {<br>
> ValueProfNode **NodesKind[IPVK_Last + 1];<br>
> } ValueProfRuntimeRecord;<br>
><br>
> -/* Forward declarations of C interfaces. */<br>
> -int initializeValueProfRuntimeRecord(ValueProfRuntimeRecord *RuntimeRecord,<br>
> - const uint16_t *NumValueSites,<br>
> - ValueProfNode **Nodes);<br>
> -void finalizeValueProfRuntimeRecord(ValueProfRuntimeRecord *RuntimeRecord);<br>
> -uint32_t getValueProfDataSizeRT(const ValueProfRuntimeRecord *Record);<br>
> -ValueProfData *<br>
> -serializeValueProfDataFromRT(const ValueProfRuntimeRecord *Record,<br>
> - ValueProfData *Dst);<br>
> -uint32_t getNumValueKindsRT(const void *R);<br>
> ValueProfRecord *getFirstValueProfRecord(ValueProfData *VPD);<br>
> ValueProfRecord *getValueProfRecordNext(ValueProfRecord *VPR);<br>
> InstrProfValueData *getValueProfRecordValueData(ValueProfRecord *VPR);<br>
> @@ -554,115 +544,6 @@ ValueProfData *serializeValueProfDataFro<br>
> return VPD;<br>
> }<br>
><br>
> -/*<br>
> - * The value profiler runtime library stores the value profile data<br>
> - * for a given function in \c NumValueSites and \c Nodes structures.<br>
> - * \c ValueProfRuntimeRecord class is used to encapsulate the runtime<br>
> - * profile data and provides fast interfaces to retrieve the profile<br>
> - * information. This interface is used to initialize the runtime record<br>
> - * and pre-compute the information needed for efficient implementation<br>
> - * of callbacks required by ValueProfRecordClosure class.<br>
> - */<br>
> -int initializeValueProfRuntimeRecord(ValueProfRuntimeRecord *RuntimeRecord,<br>
> - const uint16_t *NumValueSites,<br>
> - ValueProfNode **Nodes) {<br>
> - unsigned I, S = 0, NumValueKinds = 0;<br>
> - RuntimeRecord->NumValueSites = NumValueSites;<br>
> - RuntimeRecord->Nodes = Nodes;<br>
> - for (I = 0; I <= IPVK_Last; I++) {<br>
> - uint16_t N = NumValueSites[I];<br>
> - if (!N)<br>
> - continue;<br>
> - NumValueKinds++;<br>
> -<br>
> - RuntimeRecord->NodesKind[I] = Nodes ? &Nodes[S] : INSTR_PROF_NULLPTR;<br>
> - S += N;<br>
> - }<br>
> - RuntimeRecord->NumValueKinds = NumValueKinds;<br>
> - return 0;<br>
> -}<br>
> -<br>
> -void finalizeValueProfRuntimeRecord(ValueProfRuntimeRecord *RuntimeRecord) {}<br>
> -<br>
> -/* ValueProfRecordClosure Interface implementation for<br>
> - * ValueProfDataRuntimeRecord. */<br>
> -uint32_t getNumValueKindsRT(const void *R) {<br>
> - return ((const ValueProfRuntimeRecord *)R)->NumValueKinds;<br>
> -}<br>
> -<br>
> -uint32_t getNumValueSitesRT(const void *R, uint32_t VK) {<br>
> - return ((const ValueProfRuntimeRecord *)R)->NumValueSites[VK];<br>
> -}<br>
> -<br>
> -uint32_t getNumValueDataForSiteRT(const void *R, uint32_t VK, uint32_t S) {<br>
> - uint32_t C = 0;<br>
> - const ValueProfRuntimeRecord *Record = (const ValueProfRuntimeRecord *)R;<br>
> - ValueProfNode *Site =<br>
> - Record->NodesKind[VK] ? Record->NodesKind[VK][S] : INSTR_PROF_NULLPTR;<br>
> - while (Site) {<br>
> - C++;<br>
> - Site = Site->Next;<br>
> - }<br>
> - if (C > UCHAR_MAX)<br>
> - C = UCHAR_MAX;<br>
> -<br>
> - return C;<br>
> -}<br>
> -<br>
> -uint32_t getNumValueDataRT(const void *R, uint32_t VK) {<br>
> - unsigned I, S = 0;<br>
> - const ValueProfRuntimeRecord *Record = (const ValueProfRuntimeRecord *)R;<br>
> - for (I = 0; I < Record->NumValueSites[VK]; I++)<br>
> - S += getNumValueDataForSiteRT(Record, VK, I);<br>
> - return S;<br>
> -}<br>
> -<br>
> -void getValueForSiteRT(const void *R, InstrProfValueData *Dst, uint32_t VK,<br>
> - uint32_t S) {<br>
> - unsigned I, N = 0;<br>
> - const ValueProfRuntimeRecord *Record = (const ValueProfRuntimeRecord *)R;<br>
> - N = getNumValueDataForSiteRT(R, VK, S);<br>
> - if (N == 0)<br>
> - return;<br>
> - ValueProfNode *VNode = Record->NodesKind[VK][S];<br>
> - for (I = 0; I < N; I++) {<br>
> - Dst[I] = VNode->VData;<br>
> - VNode = VNode->Next;<br>
> - }<br>
> -}<br>
> -<br>
> -ValueProfData *allocValueProfDataRT(size_t TotalSizeInBytes) {<br>
> - return (ValueProfData *)calloc(TotalSizeInBytes, 1);<br>
> -}<br>
> -<br>
> -static ValueProfRecordClosure RTRecordClosure = {<br>
> - INSTR_PROF_NULLPTR, getNumValueKindsRT, getNumValueSitesRT,<br>
> - getNumValueDataRT, getNumValueDataForSiteRT, INSTR_PROF_NULLPTR,<br>
> - getValueForSiteRT, allocValueProfDataRT};<br>
> -<br>
> -/*<br>
> - * Return the size of ValueProfData structure to store data<br>
> - * recorded in the runtime record.<br>
> - */<br>
> -uint32_t getValueProfDataSizeRT(const ValueProfRuntimeRecord *Record) {<br>
> - RTRecordClosure.Record = Record;<br>
> - return getValueProfDataSize(&RTRecordClosure);<br>
> -}<br>
> -<br>
> -/*<br>
> - * Return a ValueProfData instance that stores the data collected<br>
> - * from runtime. If \c DstData is provided by the caller, the value<br>
> - * profile data will be store in *DstData and DstData is returned,<br>
> - * otherwise the method will allocate space for the value data and<br>
> - * return pointer to the newly allocated space.<br>
> - */<br>
> -ValueProfData *<br>
> -serializeValueProfDataFromRT(const ValueProfRuntimeRecord *Record,<br>
> - ValueProfData *DstData) {<br>
> - RTRecordClosure.Record = Record;<br>
> - return serializeValueProfDataFrom(&RTRecordClosure, DstData);<br>
> -}<br>
> -<br>
> #undef INSTR_PROF_COMMON_API_IMPL<br>
> #endif /* INSTR_PROF_COMMON_API_IMPL */<br>
><br>
><br>
> Modified: llvm/trunk/unittests/ProfileData/InstrProfTest.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ProfileData/InstrProfTest.cpp?rev=269384&r1=269383&r2=269384&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ProfileData/InstrProfTest.cpp?rev=269384&r1=269383&r2=269384&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/unittests/ProfileData/InstrProfTest.cpp (original)<br>
> +++ llvm/trunk/unittests/ProfileData/InstrProfTest.cpp Thu May 12 19:23:49 2016<br>
> @@ -659,39 +659,36 @@ TEST_P(MaybeSparseInstrProfTest, get_ica<br>
> }<br>
> }<br>
><br>
> -// Synthesize runtime value profile data.<br>
> -ValueProfNode Site1Values[5] = {{{uint64_t(callee1), 400}, &Site1Values[1]},<br>
> - {{uint64_t(callee2), 1000}, &Site1Values[2]},<br>
> - {{uint64_t(callee3), 500}, &Site1Values[3]},<br>
> - {{uint64_t(callee4), 300}, &Site1Values[4]},<br>
> - {{uint64_t(callee5), 100}, nullptr}};<br>
> -<br>
> -ValueProfNode Site2Values[4] = {{{uint64_t(callee5), 800}, &Site2Values[1]},<br>
> - {{uint64_t(callee3), 1000}, &Site2Values[2]},<br>
> - {{uint64_t(callee2), 2500}, &Site2Values[3]},<br>
> - {{uint64_t(callee1), 1300}, nullptr}};<br>
> -<br>
> -ValueProfNode Site3Values[3] = {{{uint64_t(callee6), 800}, &Site3Values[1]},<br>
> - {{uint64_t(callee3), 1000}, &Site3Values[2]},<br>
> - {{uint64_t(callee4), 5500}, nullptr}};<br>
> -<br>
> -ValueProfNode Site4Values[2] = {{{uint64_t(callee2), 1800}, &Site4Values[1]},<br>
> - {{uint64_t(callee3), 2000}, nullptr}};<br>
> -<br>
> -static ValueProfNode *ValueProfNodes[5] = {&Site1Values[0], &Site2Values[0],<br>
> - &Site3Values[0], &Site4Values[0],<br>
> - nullptr};<br>
> -<br>
> -static uint16_t NumValueSites[IPVK_Last + 1] = {5};<br>
> -TEST_P(MaybeSparseInstrProfTest, runtime_value_prof_data_read_write) {<br>
> - ValueProfRuntimeRecord RTRecord;<br>
> - initializeValueProfRuntimeRecord(&RTRecord, &NumValueSites[0],<br>
> - &ValueProfNodes[0]);<br>
> +static void addValueProfData(InstrProfRecord &Record) {<br>
> + Record.reserveSites(IPVK_IndirectCallTarget, 5);<br>
> + InstrProfValueData VD0[] = {{uint64_t(callee1), 400},<br>
> + {uint64_t(callee2), 1000},<br>
> + {uint64_t(callee3), 500},<br>
> + {uint64_t(callee4), 300},<br>
> + {uint64_t(callee5), 100}};<br>
> + Record.addValueData(IPVK_IndirectCallTarget, 0, VD0, 5, nullptr);<br>
> + InstrProfValueData VD1[] = {{uint64_t(callee5), 800},<br>
> + {uint64_t(callee3), 1000},<br>
> + {uint64_t(callee2), 2500},<br>
> + {uint64_t(callee1), 1300}};<br>
> + Record.addValueData(IPVK_IndirectCallTarget, 1, VD1, 4, nullptr);<br>
> + InstrProfValueData VD2[] = {{uint64_t(callee6), 800},<br>
> + {uint64_t(callee3), 1000},<br>
> + {uint64_t(callee4), 5500}};<br>
> + Record.addValueData(IPVK_IndirectCallTarget, 2, VD2, 3, nullptr);<br>
> + InstrProfValueData VD3[] = {{uint64_t(callee2), 1800},<br>
> + {uint64_t(callee3), 2000}};<br>
> + Record.addValueData(IPVK_IndirectCallTarget, 3, VD3, 2, nullptr);<br>
> + Record.addValueData(IPVK_IndirectCallTarget, 4, nullptr, 0, nullptr);<br>
> +}<br>
><br>
> - ValueProfData *VPData = serializeValueProfDataFromRT(&RTRecord, nullptr);<br>
> +TEST_P(MaybeSparseInstrProfTest, value_prof_data_read_write) {<br>
> + InstrProfRecord SrcRecord("caller", 0x1234, {1ULL << 31, 2});<br>
> + addValueProfData(SrcRecord);<br>
> + std::unique_ptr<ValueProfData> VPData =<br>
> + ValueProfData::serializeFrom(SrcRecord);<br>
><br>
> InstrProfRecord Record("caller", 0x1234, {1ULL << 31, 2});<br>
> -<br>
> VPData->deserializeTo(Record, nullptr);<br>
><br>
> // Now read data from Record and sanity check the data<br>
> @@ -748,18 +745,14 @@ TEST_P(MaybeSparseInstrProfTest, runtime<br>
> ASSERT_EQ(2000U, VD_3[0].Count);<br>
> ASSERT_EQ(StringRef((const char *)VD_3[1].Value, 7), StringRef("callee2"));<br>
> ASSERT_EQ(1800U, VD_3[1].Count);<br>
> -<br>
> - finalizeValueProfRuntimeRecord(&RTRecord);<br>
> - free(VPData);<br>
> }<br>
><br>
> -static uint16_t NumValueSites2[IPVK_Last + 1] = {1};<br>
> -TEST_P(MaybeSparseInstrProfTest, runtime_value_prof_data_read_write_mapping) {<br>
> - ValueProfRuntimeRecord RTRecord;<br>
> - initializeValueProfRuntimeRecord(&RTRecord, &NumValueSites2[0],<br>
> - &ValueProfNodes[0]);<br>
> +TEST_P(MaybeSparseInstrProfTest, value_prof_data_read_write_mapping) {<br>
><br>
> - ValueProfData *VPData = serializeValueProfDataFromRT(&RTRecord, nullptr);<br>
> + InstrProfRecord SrcRecord("caller", 0x1234, {1ULL << 31, 2});<br>
> + addValueProfData(SrcRecord);<br>
> + std::unique_ptr<ValueProfData> VPData =<br>
> + ValueProfData::serializeFrom(SrcRecord);<br>
><br>
> InstrProfRecord Record("caller", 0x1234, {1ULL << 31, 2});<br>
> InstrProfSymtab Symtab;<br>
> @@ -773,7 +766,7 @@ TEST_P(MaybeSparseInstrProfTest, runtime<br>
> VPData->deserializeTo(Record, &Symtab.getAddrHashMap());<br>
><br>
> // Now read data from Record and sanity check the data<br>
> - ASSERT_EQ(1U, Record.getNumValueSites(IPVK_IndirectCallTarget));<br>
> + ASSERT_EQ(5U, Record.getNumValueSites(IPVK_IndirectCallTarget));<br>
> ASSERT_EQ(5U, Record.getNumValueDataForSite(IPVK_IndirectCallTarget, 0));<br>
><br>
> auto Cmp = [](const InstrProfValueData &VD1, const InstrProfValueData &VD2) {<br>
> @@ -791,8 +784,6 @@ TEST_P(MaybeSparseInstrProfTest, runtime<br>
><br>
> // callee5 does not have a mapped value -- default to 0.<br>
> ASSERT_EQ(VD_0[4].Value, 0ULL);<br>
> - finalizeValueProfRuntimeRecord(&RTRecord);<br>
> - free(VPData);<br>
> }<br>
><br>
> TEST_P(MaybeSparseInstrProfTest, get_max_function_count) {<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
<br>
</blockquote></div><br></div>