<div dir="ltr">Done.<div><br></div><div>David</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Apr 11, 2016 at 10:12 AM, Adam Nemet <span dir="ltr"><<a href="mailto:anemet@apple.com" target="_blank">anemet@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
> On Apr 9, 2016, at 8:32 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: Sat Apr  9 22:32:02 2016<br>
> New Revision: 265890<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=265890&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=265890&view=rev</a><br>
> Log:<br>
> [PGO] Fix deserialize bug<br>
><br>
> Raw function pointer collected by value<br>
> profile data may be from external functions<br>
> that are not instrumented. They won't have<br>
> mapping data to be used by the deserializer.<br>
> Force the value to be 0 in this case.<br>
<br>
Please also add this comment to the code as well.<br>
<br>
Adam<br>
<br>
><br>
> Modified:<br>
>    llvm/trunk/lib/ProfileData/InstrProf.cpp<br>
>    llvm/trunk/unittests/ProfileData/InstrProfTest.cpp<br>
><br>
> Modified: llvm/trunk/lib/ProfileData/InstrProf.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ProfileData/InstrProf.cpp?rev=265890&r1=265889&r2=265890&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ProfileData/InstrProf.cpp?rev=265890&r1=265889&r2=265890&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/ProfileData/InstrProf.cpp (original)<br>
> +++ llvm/trunk/lib/ProfileData/InstrProf.cpp Sat Apr  9 22:32:02 2016<br>
> @@ -398,8 +398,10 @@ uint64_t InstrProfRecord::remapValue(uin<br>
>         std::lower_bound(ValueMap->begin(), ValueMap->end(), Value,<br>
>                          [](const std::pair<uint64_t, uint64_t> &LHS,<br>
>                             uint64_t RHS) { return LHS.first < RHS; });<br>
> -    if (Result != ValueMap->end())<br>
> +    if (Result != ValueMap->end() && Result->first == Value)<br>
>       Value = (uint64_t)Result->second;<br>
> +    else<br>
> +      Value = 0;<br>
>     break;<br>
>   }<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=265890&r1=265889&r2=265890&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ProfileData/InstrProfTest.cpp?rev=265890&r1=265889&r2=265890&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/unittests/ProfileData/InstrProfTest.cpp (original)<br>
> +++ llvm/trunk/unittests/ProfileData/InstrProfTest.cpp Sat Apr  9 22:32:02 2016<br>
> @@ -752,6 +752,46 @@ TEST_P(MaybeSparseInstrProfTest, runtime<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>
> +<br>
> +  ValueProfData *VPData = serializeValueProfDataFromRT(&RTRecord, nullptr);<br>
> +<br>
> +  InstrProfRecord Record("caller", 0x1234, {1ULL << 31, 2});<br>
> +  InstrProfSymtab Symtab;<br>
> +  Symtab.mapAddress(uint64_t(callee1), 0x1000ULL);<br>
> +  Symtab.mapAddress(uint64_t(callee2), 0x2000ULL);<br>
> +  Symtab.mapAddress(uint64_t(callee3), 0x3000ULL);<br>
> +  Symtab.mapAddress(uint64_t(callee4), 0x4000ULL);<br>
> +  // Missing mapping for callee5<br>
> +  Symtab.finalizeSymtab();<br>
> +<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.getNumValueDataForSite(IPVK_IndirectCallTarget, 0));<br>
> +<br>
> +  auto Cmp = [](const InstrProfValueData &VD1, const InstrProfValueData &VD2) {<br>
> +    return VD1.Count > VD2.Count;<br>
> +  };<br>
> +  std::unique_ptr<InstrProfValueData[]> VD_0(<br>
> +      Record.getValueForSite(IPVK_IndirectCallTarget, 0));<br>
> +  std::sort(&VD_0[0], &VD_0[5], Cmp);<br>
> +  ASSERT_EQ(VD_0[0].Value, 0x2000ULL);<br>
> +  ASSERT_EQ(1000U, VD_0[0].Count);<br>
> +  ASSERT_EQ(VD_0[1].Value, 0x3000ULL);<br>
> +  ASSERT_EQ(500U, VD_0[1].Count);<br>
> +  ASSERT_EQ(VD_0[2].Value, 0x1000ULL);<br>
> +  ASSERT_EQ(400U, VD_0[2].Count);<br>
> +<br>
> +  // callee5 does not have a mapped value -- default to 0.<br>
> +  ASSERT_EQ(VD_0[4].Value, 0ULL);<br>
> +}<br>
> +<br>
> TEST_P(MaybeSparseInstrProfTest, get_max_function_count) {<br>
>   InstrProfRecord Record1("foo", 0x1234, {1ULL << 31, 2});<br>
>   InstrProfRecord Record2("bar", 0, {1ULL << 63});<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>