<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Feb 18, 2016 at 8:47 AM, Akira Hatanaka <span dir="ltr"><<a href="mailto:ahatanak@gmail.com" target="_blank">ahatanak@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="">On Thu, Feb 18, 2016 at 8:10 AM, Manman Ren <span dir="ltr"><<a href="mailto:manman.ren@gmail.com" target="_blank">manman.ren@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Wed, Feb 17, 2016 at 10:33 PM, Akira Hatanaka <span dir="ltr"><<a href="mailto:ahatanak@gmail.com" target="_blank">ahatanak@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">ahatanak added a comment.<br>
<br>
OK, I now understand what you meant.<br>
<span><br>
> How about the following?<br>
<br>
><br>
<br>
>   else if (LocalAlignment == 8) {<br>
<br>
>     if (NumBytesAtAlign8 == 0) {<br>
<br>
>       // We have not seen any 8-byte aligned element yet. There is no padding and we are either 4-byte<br>
<br>
>       // aligned or 8-byte aligned depending on NumBytesAtAlign4.<br>
<br>
>       // Add in 4 bytes padding if we are not 8-byte aligned including this element.<br>
<br>
>       if ((LocalSize + NumBytesAtAlign4) % 8 != 0) {<br>
<br>
>         memmove(&Buffer[Index - 4], &Buffer[Index], NumBytesAtAlign4);<br>
<br>
>         Index -= 4;<br>
<br>
>       }<br>
<br>
<br>
</span>If Capacity is not a multiple of 8, (LocalSize + NumBytesAtAlign4) % 8 doesn't tell you whether the new element will be 8-byte aligned. For example, if Capacity==36, NumBytesAtAlign4==4, and LocalSize==8, (LocalSize + NumBytesAtAlign4) equals 12 but padding is not needed as the new element can start at Index=24.</blockquote><div><br></div></span><div>I don't quite get why the new element can start at Index of 24, does it have a LocalAlignment of 8?<br> <br></div></div></div></div></blockquote><div><br></div></span><div>This part is enclosed in "else if (LocalAlignment == 8)", so it's LocalAlignment should be 8 and it can start at Index=24 (which is 8-byte aligned), no? By new element, I meant the element that is currently being pushed.</div><span class=""><div> <br></div></span></div></div></div></blockquote><div><br></div><div>Also, in case it wasn't clear, when I say the new element (of size 8) starts at Index=24, I mean the first byte of the element is byte 24 and the last byte is byte 31.</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class=""><div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"> Note that it's possible to have a Capacity that isn't a multiple of 8 by calling TypeLocBuilder::reserve.</blockquote></span><div>Yeah, I probably missed the case where Capacity is not aligned.<br><br></div><div>Please update the patch.<span><font color="#888888"><br><br></font></span></div><span><font color="#888888"><div>Manman <br></div></font></span><span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"> I think padding is needed if the new index (Index - LocalSize) is not a multiple of 8.<br>
<br>
<br>
<a href="http://reviews.llvm.org/D16843" rel="noreferrer" target="_blank">http://reviews.llvm.org/D16843</a><br>
<br>
<br>
<br>
</blockquote></span></div><br></div></div>
</blockquote></span></div><br></div></div>
</blockquote></div><br></div></div>