[PATCH] D15057: [PGO] Enable common VP format in profile runtime

[David Li via llvm-commits]

davidxl marked an inline comment as done.

================
Comment at: lib/profile/InstrProfiling.c:70
@@ +69,3 @@
+                                   uint32_t ValueKind, uint16_t NumValueSites) {
+  *((uint16_t *)&Data->NumValueSites[ValueKind]) = NumValueSites;
+}
----------------
betulb wrote:
> Is the casting really needed? What's passed in is a __llvm_profile_data pointer, so 
> shouldn't Data->NumValueSites[ValueKind] be OK?
the const qualifier needs to be removed.

================
Comment at: lib/profile/InstrProfiling.c:224
@@ +223,3 @@
+    if (!I->Values)
+      continue;
+    ValueProfRuntimeRecord *R = &Records[I - DataBegin];
----------------
betulb wrote:
> Shouldn't line 223 check for !getValueProfDataSizeRT(R) and not for !I->Values? The new check also should happen after line 225.
You are right. I->Values is lazily initialized -- so I->Values == NULL does not mean we can skip it (only when there is zero value site statically). I need to enhance the test case to cover it.


http://reviews.llvm.org/D15057