<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Nov 24, 2015 at 11:21 AM, Xinliang David Li via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: davidxl<br>
Date: Tue Nov 24 13:21:15 2015<br>
New Revision: 254008<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=254008&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=254008&view=rev</a><br>
Log:<br>
[PGO] Introduce value profile data closure type.<br>
<br>
The closure is designed to abstact away two types of value profile<br>
data:<br>
- InstrProfRecord which is the primary data structure used to<br>
represent profile data in host tools (reader, writer, and profile-use)<br>
- value profile runtime data structure suitable to be used by C<br>
runtime library.<br>
Both sources of data need to serialize to disk/memory-buffer in common<br>
format: ValueProfData.<br>
<br>
The abstraction allows compiler-rt's raw profiler writer to share<br>
the same code with indexed profile writer.<br>
<br>
<br>
Modified:<br>
llvm/trunk/include/llvm/ProfileData/InstrProf.h<br>
<br>
Modified: llvm/trunk/include/llvm/ProfileData/InstrProf.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ProfileData/InstrProf.h?rev=254008&r1=254007&r2=254008&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ProfileData/InstrProf.h?rev=254008&r1=254007&r2=254008&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/ProfileData/InstrProf.h (original)<br>
+++ llvm/trunk/include/llvm/ProfileData/InstrProf.h Tue Nov 24 13:21:15 2015<br>
@@ -538,6 +538,18 @@ typedef struct ValueProfData {<br>
ValueProfRecord *getFirstValueProfRecord();<br>
} ValueProfData;<br>
<br>
+typedef struct ValueProfRecordClosure {<br>
+ void *Record;<br>
+ uint32_t (*GetNumValueKinds)(void *Record);<br>
+ uint32_t (*GetNumValueSites)(void *Record, uint32_t VKind);<br>
+ uint32_t (*GetNumValueData)(void *Record, uint32_t VKind);<br>
+ uint32_t (*GetNumValueDataForSite)(void *R, uint32_t VK, uint32_t S);<br></blockquote><div><br></div><div>Breaking a line here is preferable to using inconsistent names.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ uint64_t (*RemapValueData)(uint64_t Value);<br>
+ void (*GetValueForSite)(InstrProfValueData Dst[], void *R, uint32_t K,<br>
+ uint32_t S);<br></blockquote><div><br></div><div>The actual meaning of `[]` here is unintuitive (bad aspect of C language design). Just use `*` to avoid confusion.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ ValueProfData *(*AllocateValueProfData)(size_t TotalSizeInBytes);<br>
+} ValueProfRecordClosure;<br>
+<br></blockquote><div><br></div><div>Also I would suggest adding documentation to each of these function pointers. You are essentially declaring an abstract base class. It should document it as such (or reference other documentation from which it is "obvious" just based on the names, but I don't know if we have such other documentation).</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
inline uint32_t getValueProfRecordHeaderSize(uint32_t NumValueSites) {<br>
uint32_t Size = offsetof(ValueProfRecord, SiteCountArray) +<br>
sizeof(uint8_t) * NumValueSites;<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>
</blockquote></div><br></div></div>