[PATCH] LLVM changes for value profiling support

[Betul Buyukkurt]

Hi Diego,

Thanks for your comments. I'll not post an extra patch answering your comments. I'll rather answer them in the upcoming CL's that follows the below plan for dividing the patches into smaller CL's for merging. Quoting from the previous email, here is my merge plan:

1. Intrinsic instruction for value profiling.
2. Making changes to the data structures used by the readers/writers, and updating of the readers to preserve the current formats.
3. Indexed reader/writer changes for version 3 i.e. the one that's going to be used by the value profiling as well
4. Lowering of profiling intrinsic and generation of Data w/ value profiling entries. Raw reader updates for reading version 2 data.
5. compiler-rt changes for value profiling.

(4 & 5) changes should go in together not to break the builds.

6. Clang changes for generating instrumentation and attaching of new profile metadata.

7. Clang flag controls for value profiling types (enabling/disabling instrumentation/pgo use).


================
Comment at: include/llvm/ProfileData/InstrProf.h:54
@@ +53,3 @@
+    reserved_2 = 2,
+    reserved_3 = 3,
+    last = 3,
----------------
dnovillo wrote:
> Any idea on why reserving 3 slots? Why not more? Or fewer? I guess I'm asking what other kinds of value profiling do you have in mind for future changes.
I wanted to add enough reserved slots so that the Data ending is aligned at a 64 bit boundary on both 32-bit and 64-bit targets. The number of reserved slots should then either be 1 or 3 or more. Three seemed most appropriate to allow no reader changes for the future. We do have internal plans for adding further value types.

================
Comment at: include/llvm/ProfileData/InstrProfReader.h:18
@@ -17,3 +17,2 @@
 
-#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringExtras.h"
----------------
dnovillo wrote:
> Please keep this. The file still uses ArrayRefs.
ArrayRef.h is included through InstrProf.h which is two lines below.

================
Comment at: include/llvm/ProfileData/InstrProfReader.h:249
@@ +248,3 @@
+      case 2: return ReadDataV1V2(K, D, N);
+      case 3: return ReadDataV3(K, D, N);
+    }
----------------
dnovillo wrote:
> A question on format evolution. How far back should we go as the file format evolves?
I was not sure on this so I initially supported all raw and indexed file format versions in the readers. I received review comments stating that I was not supposed to preserve backwards compatibility on the raw readers but with indexed readers the readers should be backwards compatible.

================
Comment at: test/Instrumentation/InstrProfiling/linkage.ll:26
@@ -25,3 +25,3 @@
 ; CHECK: @"__llvm_profile_counters_linkage.ll:foo_internal" = internal global
-; CHECK: @"__llvm_profile_data_linkage.ll:foo_internal" = internal constant
+; CHECK: @"__llvm_profile_data_linkage.ll:foo_internal" = internal
 define internal void @foo_internal() {
----------------
dnovillo wrote:
> Any way that we could keep these variables in .rodata?
With value profiling support the __llvm_profile_data variables are no longer constant. So .rodata is not appropriate. In addition, these having their own sections also helps with the "linker magic" that is used on some platforms to detect the section start and ends.

http://reviews.llvm.org/D8908

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