[PATCH] LLVM changes for indirect call target profiling support

[David]

Betul, I have some suggestions on major restructuring of the code, so there are not many detailed review comments in this round.

David


================
Comment at: docs/LangRef.rst:8086
@@ +8085,3 @@
+
+Arguments:
+""""""""""
----------------
The argument order of the intrinsic is not consistent with the underlying runtime function. It might be better to keep them in sync.

================
Comment at: docs/LangRef.rst:8089
@@ +8088,3 @@
+
+The first argument is a pointer to a global variable containing the
+name of the entity being instrumented. This should generally be the
----------------
Can entity anything else other than function? If not, make it clear 'name of the function ...'.

================
Comment at: docs/LangRef.rst:8093
@@ +8092,3 @@
+
+The second argument is a hash value that can be used by the consumer
+of the profile data to detect changes to the instrumented source. It
----------------
Why do we need to do this at instruction level? Function level mismatch checking by the consumer should be good enough.

================
Comment at: docs/LangRef.rst:8099
@@ +8098,3 @@
+The third argument represents the kind of the value profiling that is done.
+The fourth argument refers to the value profiled at the instrumented expression.
+The last argument indicates to which of the instrumented expressions for ``name``
----------------
The fourth argument is the value of the expression being profiled.

================
Comment at: docs/LangRef.rst:8100
@@ +8099,3 @@
+The fourth argument refers to the value profiled at the instrumented expression.
+The last argument indicates to which of the instrumented expressions for ``name``
+should the profiled value be recorded. It should be >= 0.
----------------
The last argument is the index of the instrumented expression within the function.

================
Comment at: include/llvm/IR/IntrinsicInst.h:386
@@ +385,3 @@
+
+    GlobalVariable *getName() const {
+      return cast<GlobalVariable>(
----------------
Maybe GetFunctionName?

================
Comment at: include/llvm/ProfileData/InstrProfReader.h:149
@@ +148,3 @@
+    struct ProfileDataV1 V1Data;
+    const IntPtrT FunctionPointer;
+    const uint32_t NumIndirectCallSites;
----------------
Changing ProfileData structure like this will eventually lead to reader code to be come really messy and unmanageable. 

This is a simple way to avoid this change -- simply 'peel' the value profile related per-function data into a different data section. It will make adding new value profilers in the future much simpler.

================
Comment at: include/llvm/ProfileData/InstrProfReader.h:162
@@ -161,2 +161,3 @@
   };
-
+  struct RawHeaderV2 {
+    struct RawHeaderV1 V1Header;
----------------
Similarly, instead of changing the common header, simply split out the value profiler related control info into subheaders, so that the file layout looks like this:

    RawHeader
    ValueProfilerHeader1
    ValueProfilerHeader2

The version number in the main raw header indicates the presence or absense of the value profiler header in the file, and can decide to read it or skip it.

By so doing, the header reading code can be shared between different versions.

I strongly suggest you change like this ..

http://reviews.llvm.org/D8908

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/