On Thu, Feb 18, 2016 at 8:10 AM, Manman Ren <manman....@gmail.com> wrote:
> > > On Wed, Feb 17, 2016 at 10:33 PM, Akira Hatanaka <ahata...@gmail.com> > wrote: > >> ahatanak added a comment. >> >> OK, I now understand what you meant. >> >> > How about the following? >> >> > >> >> > else if (LocalAlignment == 8) { >> >> > if (NumBytesAtAlign8 == 0) { >> >> > // We have not seen any 8-byte aligned element yet. There is no >> padding and we are either 4-byte >> >> > // aligned or 8-byte aligned depending on NumBytesAtAlign4. >> >> > // Add in 4 bytes padding if we are not 8-byte aligned including >> this element. >> >> > if ((LocalSize + NumBytesAtAlign4) % 8 != 0) { >> >> > memmove(&Buffer[Index - 4], &Buffer[Index], NumBytesAtAlign4); >> >> > Index -= 4; >> >> > } >> >> >> 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. > > > I don't quite get why the new element can start at Index of 24, does it > have a LocalAlignment of 8? > > 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. > Note that it's possible to have a Capacity that isn't a multiple of 8 by >> calling TypeLocBuilder::reserve. > > Yeah, I probably missed the case where Capacity is not aligned. > > Please update the patch. > > Manman > >> I think padding is needed if the new index (Index - LocalSize) is not a >> multiple of 8. >> >> >> http://reviews.llvm.org/D16843 >> >> >> >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits