[PATCH] D10674: Value profiling - patchset 3

[Xinliang David Li via llvm-commits]

Justin, can you take another look at the patch? Most of the issues we
have raised about the value profile indexed format have been resolved
by Betul. I think this version is ready to land in trunk. For other
value profile patches, we can also choose to make it off by default
initially to minimize risks. Let me know what you think.

thanks,

David

On Thu, Aug 13, 2015 at 10:17 PM, Betul Buyukkurt <betulb at codeaurora.org> wrote:
> betulb marked 4 inline comments as done.
>
> ================
> Comment at: include/llvm/ProfileData/InstrProf.h:60
> @@ +59,3 @@
> +  // Get a pointer to internal storage of a string in set
> +  const char* getStringData(StringRef Str) {
> +    auto Result = StringValueSet.find(Str);
> ----------------
> I've modified the data structure, however I've chosen to use an std::list over a linked list. I understand the presence of the cost for sites with no value data. I may revisit if the implementation is not preferred. I think usage of list makes it easier to access, sort, be iterable, and merge. The list of pairs implementation also allows sorting either by target values or numtaken. Please inform is the code is not agreeable.
>
> ================
> Comment at: include/llvm/ProfileData/InstrProf.h:65
> @@ +64,3 @@
> +  // Insert a string to StringTable
> +  const char* insertString(StringRef Str) {
> +    auto Result = StringValueSet.insert(Str);
> ----------------
> I'm storing a const char*  to the entry in the string table in the value portion of the std::pair.
>
> At the time of the indexed profile file writing, I'm converting the const char* entries to hash values of the strings pointed to by the value field. At the time of writing the hash values are represented back as const char* into the contents of the string table.
>
>
> http://reviews.llvm.org/D10674
>
>
>