<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 10, 2016 at 11:14 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">silvas created this revision.<br>
silvas added reviewers: davidxl, xur, vsk.<br>
silvas added a subscriber: llvm-commits.<br>
silvas set the repository for this revision to rL LLVM.<br>
<br>
On PS4 we ideally will make no calls into malloc/calloc/free. Currently value profiling is the only source of such calls in libprofile, but even when the compiler emits no value profiling instrumentation we still end up making memory allocation calls.<br>
<br>
What do you guys think about adding a check like this? It is conceptually similar to the check for `VPDataGatherer` which effectively is bailing out early for a somewhat related reason (in that case, it is more generally that some hooks will be null because they aren't present, rather than just "we shouldn't call them").<br>
<br>
I've run tests on my Mac laptop and everything passes. I'll do some more thorough checking on PS4 when I'm back in the office on Thu (currently I'm at a conference and don't have easy access to my workstation / devkits).<br>
<br>
Does this make sense? </blockquote><div><br></div><div>yes.</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">As far as testing I was thinking of something like using LD_PRELOAD (and whatever the mac one is) to interpose malloc/calloc/free and verify there are no calls to them.<br></blockquote><div><br></div><div>You can probably just declare malloc/calloc statically in test and trap if it is called.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
[1] We'll need to have a separate discussion of controlling VP instrumentation (probably a decision at clang driver based on triple). On PS4 we need to have it off (at least by default) to avoid interacting with a game's dynamic memory allocation.<br></blockquote><div><br></div><div>yes -- a driver level option will be useful.</div><div><br></div><div>David</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="http://reviews.llvm.org/D20148" rel="noreferrer" target="_blank">http://reviews.llvm.org/D20148</a><br>
<br>
Files:<br>
  lib/profile/InstrProfilingWriter.c<br>
<br>
Index: lib/profile/InstrProfilingWriter.c<br>
===================================================================<br>
--- lib/profile/InstrProfilingWriter.c<br>
+++ lib/profile/InstrProfilingWriter.c<br>
@@ -91,6 +91,15 @@<br>
   return 0;<br>
 }<br>
<br>
+static int hasNoValueSites(const __llvm_profile_data *DataBegin,<br>
+                           const __llvm_profile_data *DataEnd) {<br>
+  for (; DataBegin != DataEnd; ++DataBegin)<br>
+    for (int I = 0; I <= IPVK_Last; I++)<br>
+      if (DataBegin->NumValueSites[I] != 0)<br>
+        return 0;<br>
+  return 1;<br>
+}<br>
+<br>
 #define VP_BUFFER_SIZE 8 * 1024<br>
 static int writeValueProfData(WriterCallback Writer, void *WriterCtx,<br>
                               VPGatherHookType VPDataGatherer,<br>
@@ -100,7 +109,10 @@<br>
   uint32_t BufferSz;<br>
   const __llvm_profile_data *DI = 0;<br>
<br>
-  if (!VPDataGatherer)<br>
+  /* Bail out early so that we don't attempt to call any memory allocation<br>
+   * functions.<br>
+   */<br>
+  if (!VPDataGatherer || hasNoValueSites(DataBegin, DataEnd))<br>
     return 0;<br>
<br>
   BufferSz = VPBufferSize ? VPBufferSize : VP_BUFFER_SIZE;<br>
<br>
<br>
</blockquote></div><br></div></div>