ahatanak updated this revision to Diff 47413.
ahatanak added a comment.
Rename and add variables. Add comments to make it clearer what the code is
doing.
http://reviews.llvm.org/D16843
Files:
lib/Sema/TypeLocBuilder.cpp
lib/Sema/TypeLocBuilder.h
test/SemaObjCXX/typeloc-data-alignment.mm
Index: test/SemaObjCXX/typeloc-data-alignment.mm
===================================================================
--- /dev/null
+++ test/SemaObjCXX/typeloc-data-alignment.mm
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+// Make sure this doesn't crash.
+
+@protocol P
+@end
+template <class T>
+id<P> foo(T) {
+ int i;
+ foo(i);
+}
Index: lib/Sema/TypeLocBuilder.h
===================================================================
--- lib/Sema/TypeLocBuilder.h
+++ lib/Sema/TypeLocBuilder.h
@@ -41,12 +41,18 @@
/// The inline buffer.
enum { BufferMaxAlignment = llvm::AlignOf<void*>::Alignment };
llvm::AlignedCharArray<BufferMaxAlignment, InlineCapacity> InlineBuffer;
- unsigned NumBytesAtAlign4, NumBytesAtAlign8;
+
+ // Total size of the 4-byte aligned elements that were pushed since the last
+ // 8-byte element was pushed.
+ unsigned NumBytesAtAlign4;
+
+ // Indicates whether we've pushed an 8-byte aligned element.
+ bool SeenAlign8;
public:
TypeLocBuilder()
: Buffer(InlineBuffer.buffer), Capacity(InlineCapacity),
- Index(InlineCapacity), NumBytesAtAlign4(0), NumBytesAtAlign8(0)
+ Index(InlineCapacity), NumBytesAtAlign4(0), SeenAlign8(false)
{
}
@@ -80,7 +86,8 @@
LastTy = QualType();
#endif
Index = Capacity;
- NumBytesAtAlign4 = NumBytesAtAlign8 = 0;
+ NumBytesAtAlign4 = 0;
+ SeenAlign8 = false;
}
/// \brief Tell the TypeLocBuilder that the type it is storing has been
Index: lib/Sema/TypeLocBuilder.cpp
===================================================================
--- lib/Sema/TypeLocBuilder.cpp
+++ lib/Sema/TypeLocBuilder.cpp
@@ -43,6 +43,9 @@
}
void TypeLocBuilder::grow(size_t NewCapacity) {
+ // Make sure NewCapacity is a multiple of 8.
+ NewCapacity = llvm::alignTo(NewCapacity, 8);
+
assert(NewCapacity > Capacity);
// Allocate the new buffer and copy the old data into it.
@@ -68,66 +71,58 @@
LastTy = T;
#endif
- assert(LocalAlignment <= BufferMaxAlignment && "Unexpected alignment");
+ assert(Capacity % 8 == 0 && "Capacity must be a multiple of 8");
+ assert(LocalSize % 4 == 0 && "LocalSize must be a multiple of 4");
+ assert(LocalAlignment <= BufferMaxAlignment &&
+ (LocalAlignment == 8 || LocalAlignment == 4 || LocalSize == 0) &&
+ "Unexpected alignment");
+ assert(Index % 4 == 0 && "Index must be a multiple of 4");
+
+ // Check whether there is a 4-byte padding at the tail of the 4-byte elements.
+ bool CurrentlyHasPadding = SeenAlign8 && NumBytesAtAlign4 % 8;
+ bool RemoveOrInsertPadding = false;
+ unsigned RequiredCapacity = Capacity - Index + LocalSize;
+
+ // Check if the new Index has to be aligned on an 8-byte boundary.
+ bool RequiresAlign8 = SeenAlign8 || LocalAlignment == 8 && LocalSize != 0;
+
+ // Adjust the new Index if 8-byte alignment is required. Since Capacity is
+ // always a multiple of 8 and Index = Capacity - RequiredCapacity, this means
+ // RequiredCapacity has to be a multiple of 8 too.
+ if (RequiresAlign8 && RequiredCapacity % 8) {
+ // Adjust RequiredCapacity.
+ RequiredCapacity += (CurrentlyHasPadding ? -4 : 4);
+
+ // Determine if we have to insert or remove padding and move the 4-byte
+ // aligned elements that were pushed since the last 8-byte aligned
+ // element was pushed.
+ RemoveOrInsertPadding = NumBytesAtAlign4 != 0;
+ }
+
+ unsigned NewCapacity = Capacity;
// If we need to grow, grow by a factor of 2.
- if (LocalSize > Index) {
- size_t RequiredCapacity = Capacity + (LocalSize - Index);
- size_t NewCapacity = Capacity * 2;
- while (RequiredCapacity > NewCapacity)
- NewCapacity *= 2;
+ while (RequiredCapacity > NewCapacity)
+ NewCapacity *= 2;
+
+ if (NewCapacity != Capacity)
grow(NewCapacity);
- }
- // Because we're adding elements to the TypeLoc backwards, we have to
- // do some extra work to keep everything aligned appropriately.
- // FIXME: This algorithm is a absolute mess because every TypeLoc returned
- // needs to be valid. Partial TypeLocs are a terrible idea.
- // FIXME: 4 and 8 are sufficient at the moment, but it's pretty ugly to
- // hardcode them.
- if (LocalAlignment == 4) {
- if (NumBytesAtAlign8 == 0) {
- NumBytesAtAlign4 += LocalSize;
- } else {
- unsigned Padding = NumBytesAtAlign4 % 8;
- if (Padding == 0) {
- if (LocalSize % 8 == 0) {
- // Everything is set: there's no padding and we don't need to add
- // any.
- } else {
- assert(LocalSize % 8 == 4);
- // No existing padding; add in 4 bytes padding
- memmove(&Buffer[Index - 4], &Buffer[Index], NumBytesAtAlign4);
- Index -= 4;
- }
- } else {
- assert(Padding == 4);
- if (LocalSize % 8 == 0) {
- // Everything is set: there's 4 bytes padding and we don't need
- // to add any.
- } else {
- assert(LocalSize % 8 == 4);
- // There are 4 bytes padding, but we don't need any; remove it.
- memmove(&Buffer[Index + 4], &Buffer[Index], NumBytesAtAlign4);
- Index += 4;
- }
- }
- NumBytesAtAlign4 += LocalSize;
- }
- } else if (LocalAlignment == 8) {
- if (!NumBytesAtAlign8 && NumBytesAtAlign4 % 8 != 0) {
- // No existing padding and misaligned members; add in 4 bytes padding
+ if (RemoveOrInsertPadding) {
+ if (CurrentlyHasPadding) // Remove padding.
+ memmove(&Buffer[Index + 4], &Buffer[Index], NumBytesAtAlign4);
+ else // Insert padding.
memmove(&Buffer[Index - 4], &Buffer[Index], NumBytesAtAlign4);
- Index -= 4;
- }
- // Forget about any padding.
+ }
+
+ if (LocalAlignment == 8) {
+ SeenAlign8 = SeenAlign8 || LocalSize != 0;
NumBytesAtAlign4 = 0;
- NumBytesAtAlign8 += LocalSize;
- } else {
- assert(LocalSize == 0);
+ } else if (LocalAlignment == 4) {
+ NumBytesAtAlign4 += LocalSize;
}
- Index -= LocalSize;
+ Index = Capacity - RequiredCapacity;
assert(Capacity - Index == TypeLoc::getFullDataSizeForType(T) &&
"incorrect data size provided to CreateTypeSourceInfo!");
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
