<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Dec 28, 2016, at 5:22 PM, Teresa Johnson <<a href="mailto:tejohnson@google.com" class="">tejohnson@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Wed, Dec 28, 2016 at 5:07 PM, Mehdi AMINI via Phabricator <span dir="ltr" class=""><<a href="mailto:reviews@reviews.llvm.org" target="_blank" class="">reviews@reviews.llvm.org</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">mehdi_amini added a comment.<br class="">
<br class="">
FYI: had trouble with:<br class="">
<br class="">
1. Blocks that contains no record (or very little): the placeholder may not be flushed yet and the back patch will fail. I added a threshold optimization which workaround this (r290690)<br class=""></blockquote><div class=""> </div><div class="">ExitBlock always calls FlushToWord. So maybe this is a good reason to put the metadata index in its own block. As a side effect, the offset then can be in number of 32-bit words (as for the VST offset), and you only need a 32-bit index offset.</div><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
2. On 32 bits platform, we can't have fixed fields of 64 bits, I reverted the change in EmitAbbreviatedField from this patch, and instead emit now 2 fields of 32 bits each (r290693)<br class=""></blockquote><div class=""><br class=""></div><div class="">I think Emit64 should just be removed then...it didn't have any callers before your original patch, and this may be why. </div></div></div></div></div></blockquote><div><br class=""></div><div>This seems safe though, it is emitting the 64bits value as two 32 bits one.</div><div>I don’t think there is good reason for the 32 bits limits but it is just not handled by all the pieces of the implementation (in my case it was breaking in the reader).</div><div><br class=""></div><div>I removed Emit64 in r290701.</div><div><br class=""></div><div>— </div><div>Mehdi</div><div><br class=""></div></div></body></html>