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

[Betul Buyukkurt via llvm-commits]

betulb added inline comments.

================
Comment at: lib/profile/InstrProfiling.c:224
@@ +223,3 @@
+    if (!I->Values)
+      continue;
+    ValueProfRuntimeRecord *R = &Records[I - DataBegin];
----------------
davidxl wrote:
> 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.
I'm not sure if I'm following your concern about lazy initialization for I->Values.  
The earlier loop initializes the ValueProfRuntimeRecord. 

It did seemed to me that you're concerned about value data being dropped at file write time. If there is any scenario which can cause dropping of the value data in the previous implementation, then this new implementation could cause a buffer overwrite under the same circumstances, if a NULL entry for I->Values gets updated after the null check on line 194 and before line 223. But, I do not see any allowance for that at the moment.


http://reviews.llvm.org/D15057