<div dir="ltr">Still looking at the patch but:<div><br></div><div><div>    Using hash values with the most significant bit set exposed a bug where</div><div>    unsigned values were read using strtol and strtoll.  Changed to strtoul</div>
<div>    and strtoull to keep the tests passing.</div></div><div><br></div><div>This bit is clearly correct, and I'm happy for it to get exercised once the final bits land. I would split this out into its own patch and go ahead and commit it. The hash change is better as a separate patch IMO.</div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Apr 10, 2014 at 11:45 AM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra">Sorry for delays, took vacation around EuroLLVM... I'll try to catch up on some of these, but will probably only hit full steam again next week.</div>
<div class="gmail_extra"><div class=""><br>
<div class="gmail_quote">On Wed, Mar 26, 2014 at 11:27 PM, Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div style="overflow:hidden">The hash here (Working) is just like any other number in IRGen.  Its byte-<br>
swapping requirements are no different from, e.g.,<br>
CodeGenPGO::NumRegionCounters.<br>
<br>
Here’s the flow (with the current file formats):<br>
<br>
  - IRGen emits an i64 as part of the __llvm_profile_data_foo aggregate.<br>
<br>
  - CodeGen emits it in the right section, byte swapping if host!=target.<br>
<br>
  - The profile runtime dumps it in the raw binary format.  This code is in<br>
    compiler-rt:lib/profile/InstrProfilingFile.c and<br>
    compiler-rt:lib/profile/InstrProfilingBuffer.c.<br>
<br>
  - llvm-profdata reads it in, byte swapping if host!=target.  This code is<br>
    in llvm:lib/ProfileData/InstrProfReader.cpp.</div></blockquote></div><br></div>Ahh, ok. The part I was missing here was that the raw format is target-endian rather than endian neutral. That makes a lot of sense (for the same reason I originally pushed for having the raw format be different from the merged final format), but it just didn't click initially that this is why everything works.</div>

<div class="gmail_extra"><br></div><div class="gmail_extra">I'll take another look at the patch.</div></div>
</blockquote></div><br></div>