[llvm-commits] [dragonegg] r90075 - in /dragonegg/trunk: llvm-abi.h llvm-backend.cpp llvm-convert.cpp llvm-internal.h llvm-types.cpp
Duncan Sands
baldrick at free.fr
Sun Nov 29 02:14:53 PST 2009
Author: baldrick
Date: Sun Nov 29 04:14:52 2009
New Revision: 90075
URL: http://llvm.org/viewvc/llvm-project?rev=90075&view=rev
Log:
The following testcase crashes dragonegg (and llvm-gcc):
typedef __attribute__((aligned(16))) struct {
unsigned long long w[3];
} UINT192;
UINT192 ten2mk192M[] = {
{{0xcddd6e04c0592104ULL, 0x0fcf80dc33721d53ULL, 0xa7c5ac471b478423ULL}},
{{0xcddd6e04c0592104ULL, 0x0fcf80dc33721d53ULL, 0xa7c5ac471b478423ULL}},
{{0xcddd6e04c0592104ULL, 0x0fcf80dc33721d53ULL, 0xa7c5ac471b478423ULL}}
};
The reason is that gcc gives the array a size which is 8 bytes longer than
what you would expect by multiplying the element size by the array length.
It turns out that when the user increases the alignment of a type, then it
does not round the size of the type by the user alignment, but if you form
an array of that type then it does round the size of the array by the user
alignment given to the array element. Go figure. Clang people might want
to note that clang and gcc thus disagree as to what sizeof(ten2mk192M) is.
This patch changes the way array types are converted: if the LLVM array is
smaller than the gcc array, then it wraps the array in a struct with some
padding after it to get the size right. Various places then need to be
tweaked so as not to crash or give wrong results when this happens. The
testcase is reduced from bid128.c in gcc-4.5.
Modified:
dragonegg/trunk/llvm-abi.h
dragonegg/trunk/llvm-backend.cpp
dragonegg/trunk/llvm-convert.cpp
dragonegg/trunk/llvm-internal.h
dragonegg/trunk/llvm-types.cpp
Modified: dragonegg/trunk/llvm-abi.h
URL: http://llvm.org/viewvc/llvm-project/dragonegg/trunk/llvm-abi.h?rev=90075&r1=90074&r2=90075&view=diff
==============================================================================
--- dragonegg/trunk/llvm-abi.h (original)
+++ dragonegg/trunk/llvm-abi.h Sun Nov 29 04:14:52 2009
@@ -537,6 +537,9 @@
(TREE_CODE(type) == QUAL_UNION_TYPE)) {
HandleUnion(type, ScalarElts);
} else if (TREE_CODE(type) == ARRAY_TYPE) {
+ // Array with padding?
+ if (isa<StructType>(Ty))
+ Ty = cast<StructType>(Ty)->getTypeAtIndex(0U);
const ArrayType *ATy = cast<ArrayType>(Ty);
for (unsigned i = 0, e = ATy->getNumElements(); i != e; ++i) {
C.EnterField(i, Ty);
Modified: dragonegg/trunk/llvm-backend.cpp
URL: http://llvm.org/viewvc/llvm-project/dragonegg/trunk/llvm-backend.cpp?rev=90075&r1=90074&r2=90075&view=diff
==============================================================================
--- dragonegg/trunk/llvm-backend.cpp (original)
+++ dragonegg/trunk/llvm-backend.cpp Sun Nov 29 04:14:52 2009
@@ -276,10 +276,8 @@
// is the same as that of the given GCC declaration.
static bool SizeOfGlobalMatchesDecl(GlobalValue *GV, tree decl) {
const Type *Ty = GV->getType()->getElementType();
- if (!DECL_SIZE(decl) || !Ty->isSized())
+ if (!isInt64(DECL_SIZE(decl), true) || !Ty->isSized())
return true;
- if (!isInt64(DECL_SIZE(decl), true))
- return false;
return TheTarget->getTargetData()->getTypeAllocSizeInBits(Ty) ==
getInt64(DECL_SIZE(decl), true);
}
Modified: dragonegg/trunk/llvm-convert.cpp
URL: http://llvm.org/viewvc/llvm-project/dragonegg/trunk/llvm-convert.cpp?rev=90075&r1=90074&r2=90075&view=diff
==============================================================================
--- dragonegg/trunk/llvm-convert.cpp (original)
+++ dragonegg/trunk/llvm-convert.cpp Sun Nov 29 04:14:52 2009
@@ -123,19 +123,19 @@
/// true, returns whether the value is non-negative and fits in a uint64_t.
/// Always returns false for overflowed constants.
bool isInt64(tree t, bool Unsigned) {
+ if (!t)
+ return false;
if (HOST_BITS_PER_WIDE_INT == 64)
return host_integerp(t, Unsigned) && !TREE_OVERFLOW (t);
- else {
- assert(HOST_BITS_PER_WIDE_INT == 32 &&
- "Only 32- and 64-bit hosts supported!");
- return
- (TREE_CODE (t) == INTEGER_CST && !TREE_OVERFLOW (t))
- && ((TYPE_UNSIGNED(TREE_TYPE(t)) == Unsigned) ||
- // If the constant is signed and we want an unsigned result, check
- // that the value is non-negative. If the constant is unsigned and
- // we want a signed result, check it fits in 63 bits.
- (HOST_WIDE_INT)TREE_INT_CST_HIGH(t) >= 0);
- }
+ assert(HOST_BITS_PER_WIDE_INT == 32 &&
+ "Only 32- and 64-bit hosts supported!");
+ return
+ (TREE_CODE (t) == INTEGER_CST && !TREE_OVERFLOW (t))
+ && ((TYPE_UNSIGNED(TREE_TYPE(t)) == Unsigned) ||
+ // If the constant is signed and we want an unsigned result, check
+ // that the value is non-negative. If the constant is unsigned and
+ // we want a signed result, check it fits in 63 bits.
+ (HOST_WIDE_INT)TREE_INT_CST_HIGH(t) >= 0);
}
/// getInt64 - Extract the value of an INTEGER_CST as a 64 bit integer. If
@@ -1746,22 +1746,9 @@
// Variable of fixed size that goes on the stack.
Ty = ConvertType(type);
} else {
- // Dynamic-size object: must push space on the stack.
- if (TREE_CODE(type) == ARRAY_TYPE
- && isSequentialCompatible(type)
- && TYPE_SIZE(type) == DECL_SIZE(decl)) {
- Ty = ConvertType(TREE_TYPE(type)); // Get array element type.
- // Compute the number of elements in the array.
- Size = Emit(DECL_SIZE(decl), 0);
- assert(!integer_zerop(TYPE_SIZE(TREE_TYPE(type)))
- && "Array of positive size with elements of zero size!");
- Value *EltSize = Emit(TYPE_SIZE(TREE_TYPE(type)), 0);
- Size = Builder.CreateUDiv(Size, EltSize, "len");
- } else {
- // Compute the variable's size in bytes.
- Size = Emit(DECL_SIZE_UNIT(decl), 0);
- Ty = Type::getInt8Ty(Context);
- }
+ // Compute the variable's size in bytes.
+ Size = Emit(DECL_SIZE_UNIT(decl), 0);
+ Ty = Type::getInt8Ty(Context);
Size = Builder.CreateIntCast(Size, Type::getInt32Ty(Context),
/*isSigned*/false);
}
@@ -5959,15 +5946,14 @@
// If we are indexing over a fixed-size type, just use a GEP.
if (isSequentialCompatible(ArrayTreeType)) {
- Value *Idx[2];
- Idx[0] = ConstantInt::get(IntPtrTy, 0);
- Idx[1] = IndexVal;
+ // Avoid any assumptions about how the array type is represented in LLVM by
+ // doing the GEP on a pointer to the first array element.
+ const Type *EltTy = ConvertType(ElementType);
+ ArrayAddr = Builder.CreateBitCast(ArrayAddr, EltTy->getPointerTo());
Value *Ptr = POINTER_TYPE_OVERFLOW_UNDEFINED ?
- Builder.CreateInBoundsGEP(ArrayAddr, Idx, Idx + 2) :
- Builder.CreateGEP(ArrayAddr, Idx, Idx + 2);
-
- const Type *ElementTy = ConvertType(ElementType);
- unsigned Alignment = MinAlign(ArrayAlign, TD.getABITypeAlignment(ElementTy));
+ Builder.CreateInBoundsGEP(ArrayAddr, IndexVal) :
+ Builder.CreateGEP(ArrayAddr, IndexVal);
+ unsigned Alignment = MinAlign(ArrayAlign, TD.getABITypeAlignment(EltTy));
return LValue(Builder.CreateBitCast(Ptr,
PointerType::getUnqual(ConvertType(TREE_TYPE(exp)))),
Alignment);
@@ -7496,8 +7482,7 @@
// Zero length array.
if (ResultElts.empty())
- return ConstantArray::get(
- cast<ArrayType>(ConvertType(TREE_TYPE(exp))), ResultElts);
+ return Constant::getNullValue(ConvertType(TREE_TYPE(exp)));
assert(SomeVal && "If we had some initializer, we should have some value!");
// Do a post-pass over all of the elements. We're taking care of two things
@@ -7522,10 +7507,23 @@
return ConstantVector::get(ResultElts);
}
- if (AllEltsSameType)
- return ConstantArray::get(
- ArrayType::get(ElTy, ResultElts.size()), ResultElts);
- return ConstantStruct::get(Context, ResultElts, false);
+ Constant *Res = AllEltsSameType ?
+ ConstantArray::get(ArrayType::get(ElTy, ResultElts.size()), ResultElts) :
+ ConstantStruct::get(Context, ResultElts, false);
+
+ // If the array does not require extra padding, return it.
+ int64_t PadBits = getInt64(TYPE_SIZE(InitType), true) -
+ getTargetData().getTypeAllocSizeInBits(Res->getType());
+ assert(PadBits >= 0 && "Supersized array initializer!");
+ if (PadBits <= 0)
+ return Res;
+
+ // Wrap the array in a struct with padding at the end.
+ Constant *PadElts[2];
+ PadElts[0] = Res;
+ PadElts[1] = UndefValue::get(ArrayType::get(Type::getInt8Ty(Context),
+ PadBits / 8));
+ return ConstantStruct::get(Context, PadElts, 2, false);
}
@@ -8180,8 +8178,6 @@
assert(isSequentialCompatible(TREE_TYPE(Array)) &&
"Global with variable size?");
- Constant *ArrayAddr;
-
// First subtract the lower bound, if any, in the type of the index.
Constant *IndexVal = Convert(Index);
tree LowerBound = array_ref_low_bound(exp);
@@ -8190,19 +8186,19 @@
TheFolder->CreateSub(IndexVal, Convert(LowerBound)) :
TheFolder->CreateNSWSub(IndexVal, Convert(LowerBound));
- ArrayAddr = EmitLV(Array);
-
const Type *IntPtrTy = getTargetData().getIntPtrType(Context);
IndexVal = TheFolder->CreateIntCast(IndexVal, IntPtrTy,
/*isSigned*/!TYPE_UNSIGNED(IndexType));
- Value *Idx[2];
- Idx[0] = ConstantInt::get(IntPtrTy, 0);
- Idx[1] = IndexVal;
+ // Avoid any assumptions about how the array type is represented in LLVM by
+ // doing the GEP on a pointer to the first array element.
+ Constant *ArrayAddr = EmitLV(Array);
+ const Type *EltTy = ConvertType(TREE_TYPE(TREE_TYPE(Array)));
+ ArrayAddr = TheFolder->CreateBitCast(ArrayAddr, EltTy->getPointerTo());
return POINTER_TYPE_OVERFLOW_UNDEFINED ?
- TheFolder->CreateInBoundsGetElementPtr(ArrayAddr, Idx, 2) :
- TheFolder->CreateGetElementPtr(ArrayAddr, Idx, 2);
+ TheFolder->CreateInBoundsGetElementPtr(ArrayAddr, &IndexVal, 1) :
+ TheFolder->CreateGetElementPtr(ArrayAddr, &IndexVal, 1);
}
Constant *TreeConstantToLLVM::EmitLV_COMPONENT_REF(tree exp) {
Modified: dragonegg/trunk/llvm-internal.h
URL: http://llvm.org/viewvc/llvm-project/dragonegg/trunk/llvm-internal.h?rev=90075&r1=90074&r2=90075&view=diff
==============================================================================
--- dragonegg/trunk/llvm-internal.h (original)
+++ dragonegg/trunk/llvm-internal.h Sun Nov 29 04:14:52 2009
@@ -216,7 +216,7 @@
/// isInt64 - Return true if t is an INTEGER_CST that fits in a 64 bit integer.
/// If Unsigned is false, returns whether it fits in a int64_t. If Unsigned is
/// true, returns whether the value is non-negative and fits in a uint64_t.
-/// Always returns false for overflowed constants.
+/// Always returns false for overflowed constants or if t is NULL.
bool isInt64(tree_node *t, bool Unsigned);
/// getInt64 - Extract the value of an INTEGER_CST as a 64 bit integer. If
@@ -234,13 +234,42 @@
/// type and the corresponding LLVM SequentialType lay out their components
/// identically in memory, so doing a GEP accesses the right memory location.
/// We assume that objects without a known size do not.
-bool isSequentialCompatible(tree_node *type);
+inline bool isSequentialCompatible(tree_node *type) {
+ assert((TREE_CODE(type) == ARRAY_TYPE ||
+ TREE_CODE(type) == POINTER_TYPE ||
+ TREE_CODE(type) == REFERENCE_TYPE) && "not a sequential type!");
+ // This relies on gcc types with constant size mapping to LLVM types with the
+ // same size. It is possible for the component type not to have a size:
+ // struct foo; extern foo bar[];
+ return isInt64(TYPE_SIZE(TREE_TYPE(type)), true);
+}
/// OffsetIsLLVMCompatible - Return true if the given field is offset from the
/// start of the record by a constant amount which is not humongously big.
inline bool OffsetIsLLVMCompatible(tree_node *field_decl) {
- return DECL_FIELD_OFFSET(field_decl) &&
- isInt64(DECL_FIELD_OFFSET(field_decl), true);
+ return isInt64(DECL_FIELD_OFFSET(field_decl), true);
+}
+
+/// ArrayLengthOf - Returns the length of the given gcc array type, or ~0ULL if
+/// the array has variable or unknown length.
+inline uint64_t ArrayLengthOf(tree_node *type) {
+ assert(TREE_CODE(type) == ARRAY_TYPE && "Only for array types!");
+ // If the element type has variable size and the array type has variable
+ // length, but by some miracle the product gives a constant size, then we
+ // also return ~0ULL here. I can live with this, and I bet you can too!
+ if (!isInt64(TYPE_SIZE(type), true) ||
+ !isInt64(TYPE_SIZE(TREE_TYPE(type)), true))
+ return ~0ULL;
+ // May return zero for arrays that gcc considers to have non-zero length, but
+ // only if the array type has zero size (this can happen if the element type
+ // has zero size), in which case the discrepancy doesn't matter.
+ //
+ // If the user increased the alignment of the element type, then the size of
+ // the array type is rounded up by that alignment, but the size of the element
+ // is not. Since gcc requires the user alignment to be strictly smaller than
+ // the element size, this does not impact the length computation.
+ return integer_zerop(TYPE_SIZE(type)) ? 0 : getInt64(TYPE_SIZE(type), true) /
+ getInt64(TYPE_SIZE(TREE_TYPE(type)), true);
}
/// isBitfield - Returns whether to treat the specified field as a bitfield.
Modified: dragonegg/trunk/llvm-types.cpp
URL: http://llvm.org/viewvc/llvm-project/dragonegg/trunk/llvm-types.cpp?rev=90075&r1=90074&r2=90075&view=diff
==============================================================================
--- dragonegg/trunk/llvm-types.cpp (original)
+++ dragonegg/trunk/llvm-types.cpp Sun Nov 29 04:14:52 2009
@@ -296,21 +296,6 @@
}
}
-/// isSequentialCompatible - Return true if the specified gcc array or pointer
-/// type and the corresponding LLVM SequentialType lay out their components
-/// identically in memory, so doing a GEP accesses the right memory location.
-/// We assume that objects without a known size do not.
-bool isSequentialCompatible(tree_node *type) {
- assert((TREE_CODE(type) == ARRAY_TYPE ||
- TREE_CODE(type) == POINTER_TYPE ||
- TREE_CODE(type) == REFERENCE_TYPE) && "not a sequential type!");
- // This relies on gcc types with constant size mapping to LLVM types with the
- // same size. It is possible for the component type not to have a size:
- // struct foo; extern foo bar[];
- return TYPE_SIZE(TREE_TYPE(type)) &&
- isInt64(TYPE_SIZE(TREE_TYPE(type)), true);
-}
-
/// isBitfield - Returns whether to treat the specified field as a bitfield.
bool isBitfield(tree_node *field_decl) {
tree type = DECL_BIT_FIELD_TYPE(field_decl);
@@ -324,7 +309,7 @@
// Does not start on a byte boundary - must treat as a bitfield.
return true;
- if (!TYPE_SIZE(type) || !isInt64(TYPE_SIZE (type), true))
+ if (!isInt64(TYPE_SIZE (type), true))
// No size or variable sized - play safe, treat as a bitfield.
return true;
@@ -537,12 +522,8 @@
// If the type does not overlap, don't bother checking below.
- if (!TYPE_SIZE(type))
- // C-style variable length array? Be conservative.
- return true;
-
if (!isInt64(TYPE_SIZE(type), true))
- // Negative size (!) or huge - be conservative.
+ // No size, negative size (!) or huge - be conservative.
return true;
if (!getInt64(TYPE_SIZE(type), true) ||
@@ -570,8 +551,10 @@
return true;
case ARRAY_TYPE: {
- unsigned EltSizeBits = TREE_INT_CST_LOW(TYPE_SIZE(TREE_TYPE(type)));
- unsigned NumElts = cast<ArrayType>(ConvertType(type))->getNumElements();
+ uint64_t NumElts = ArrayLengthOf(type);
+ if (NumElts == ~0ULL)
+ return true;
+ unsigned EltSizeBits = getInt64(TYPE_SIZE(TREE_TYPE(type)), true);
// Check each element for overlap. This is inelegant, but effective.
for (unsigned i = 0; i != NumElts; ++i)
@@ -849,51 +832,31 @@
if ((Ty = GET_TYPE_LLVM(type)))
return Ty;
- uint64_t ElementSize;
- const Type *ElementTy;
- if (isSequentialCompatible(type)) {
- // The gcc element type maps to an LLVM type of the same size.
- // Convert to an LLVM array of the converted element type.
- ElementSize = getInt64(TYPE_SIZE(TREE_TYPE(type)), true);
- ElementTy = ConvertType(TREE_TYPE(type));
- } else {
- // The gcc element type has no size, or has variable size. Convert to an
- // LLVM array of bytes. In the unlikely but theoretically possible case
- // that the gcc array type has constant size, using an i8 for the element
- // type ensures we can produce an LLVM array of the right size.
- ElementSize = 8;
- ElementTy = Type::getInt8Ty(Context);
- }
-
- uint64_t NumElements;
- if (!TYPE_SIZE(type)) {
- // We get here if we have something that is declared to be an array with
- // no dimension. This just becomes a zero length array of the element
- // type, so 'int X[]' becomes '%X = external global [0 x i32]'.
- //
- // Note that this also affects new expressions, which return a pointer
- // to an unsized array of elements.
- NumElements = 0;
- } else if (!isInt64(TYPE_SIZE(type), true)) {
- // This handles cases like "int A[n]" which have a runtime constant
- // number of elements, but is a compile-time variable. Since these
- // are variable sized, we represent them as [0 x type].
- NumElements = 0;
- } else if (integer_zerop(TYPE_SIZE(type))) {
- // An array of zero length, or with an element type of zero size.
- // Turn it into a zero length array of the element type.
+ const Type *ElementTy = ConvertType(TREE_TYPE(type));
+ uint64_t NumElements = ArrayLengthOf(type);
+
+ if (NumElements == ~0ULL) // Variable length array?
NumElements = 0;
- } else {
- // Normal constant-size array.
- assert(ElementSize
- && "Array of positive size with elements of zero size!");
- NumElements = getInt64(TYPE_SIZE(type), true);
- assert(!(NumElements % ElementSize)
- && "Array size is not a multiple of the element size!");
- NumElements /= ElementSize;
+
+ // Create the array type.
+ Ty = ArrayType::get(ElementTy, NumElements);
+
+ // If the user increased the alignment of the array element type, then the
+ // size of the array is rounded up by that alignment even though the size
+ // of the array element type is not (!). Correct for this if necessary by
+ // adding padding. May also need padding if the element type has variable
+ // size and the array type has variable length, but by a miracle the product
+ // gives a constant size.
+ if (isInt64(TYPE_SIZE(type), true)) {
+ uint64_t PadBits = getInt64(TYPE_SIZE(type), true) -
+ getTargetData().getTypeAllocSizeInBits(Ty);
+ if (PadBits) {
+ const Type *Padding = ArrayType::get(Type::getInt8Ty(Context), PadBits / 8);
+ Ty = StructType::get(Context, Ty, Padding, NULL);
+ }
}
- Ty = TypeDB.setType(type, ArrayType::get(ElementTy, NumElements));
+ Ty = TypeDB.setType(type, Ty);
break;
}
More information about the llvm-commits
mailing list