From a0fcc08e6542a0376917b5c76a0af3eb2650c535 Mon Sep 17 00:00:00 2001 From: Duncan Sands Date: Wed, 4 Jun 2008 08:21:45 +0000 Subject: [PATCH] Change packed struct layout so that field sizes are the same as in unpacked structs, only field positions differ. This only matters for structs containing x86 long double or an apint; it may cause backwards compatibility problems if someone has bitcode containing a packed struct with a field of one of those types. The issue is that only 10 bytes are needed to hold an x86 long double: the store size is 10 bytes, but the ABI size is 12 or 16 bytes (linux/ darwin) which comes from rounding the store size up by the alignment. Because it seemed silly not to pack an x86 long double into 10 bytes in a packed struct, this is what was done. I now think this was a mistake. Reserving the ABI size for an x86 long double field even in a packed struct makes things more uniform: the ABI size is now always used when reserving space for a type. This means that developers are less likely to make mistakes. It also makes life easier for the CBE which otherwise could not represent all LLVM packed structs (PR2402). Front-end people might need to adjust the way they create LLVM structs - see following change to llvm-gcc. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@51928 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/CodeGen/AsmPrinter.h | 3 +-- lib/CodeGen/AsmPrinter.cpp | 16 +++++++--------- lib/Target/TargetData.cpp | 7 ++----- lib/Transforms/Scalar/ScalarReplAggregates.cpp | 12 +++++------- test/Other/2008-06-04-FieldSizeInPacked.ll | 14 ++++++++++++++ 5 files changed, 29 insertions(+), 23 deletions(-) create mode 100644 test/Other/2008-06-04-FieldSizeInPacked.ll diff --git a/include/llvm/CodeGen/AsmPrinter.h b/include/llvm/CodeGen/AsmPrinter.h index dd42ce141a3..e92c8a8df53 100644 --- a/include/llvm/CodeGen/AsmPrinter.h +++ b/include/llvm/CodeGen/AsmPrinter.h @@ -314,8 +314,7 @@ namespace llvm { void EmitConstantValueOnly(const Constant *CV); /// EmitGlobalConstant - Print a general LLVM constant to the .s file. - /// If Packed is false, pad to the ABI size. - void EmitGlobalConstant(const Constant* CV, bool Packed = false); + void EmitGlobalConstant(const Constant* CV); virtual void EmitMachineConstantPoolValue(MachineConstantPoolValue *MCPV); diff --git a/lib/CodeGen/AsmPrinter.cpp b/lib/CodeGen/AsmPrinter.cpp index 3265a99c05d..4ac8026a1b2 100644 --- a/lib/CodeGen/AsmPrinter.cpp +++ b/lib/CodeGen/AsmPrinter.cpp @@ -889,11 +889,9 @@ void AsmPrinter::EmitString(const ConstantArray *CVA) const { } /// EmitGlobalConstant - Print a general LLVM constant to the .s file. -/// If Packed is false, pad to the ABI size. -void AsmPrinter::EmitGlobalConstant(const Constant *CV, bool Packed) { +void AsmPrinter::EmitGlobalConstant(const Constant *CV) { const TargetData *TD = TM.getTargetData(); - unsigned Size = Packed ? - TD->getTypeStoreSize(CV->getType()) : TD->getABITypeSize(CV->getType()); + unsigned Size = TD->getABITypeSize(CV->getType()); if (CV->isNullValue() || isa(CV)) { EmitZeros(Size); @@ -903,7 +901,7 @@ void AsmPrinter::EmitGlobalConstant(const Constant *CV, bool Packed) { EmitString(CVA); } else { // Not a string. Print the values in successive locations for (unsigned i = 0, e = CVA->getNumOperands(); i != e; ++i) - EmitGlobalConstant(CVA->getOperand(i), false); + EmitGlobalConstant(CVA->getOperand(i)); } return; } else if (const ConstantStruct *CVS = dyn_cast(CV)) { @@ -914,13 +912,13 @@ void AsmPrinter::EmitGlobalConstant(const Constant *CV, bool Packed) { const Constant* field = CVS->getOperand(i); // Check if padding is needed and insert one or more 0s. - uint64_t fieldSize = TD->getTypeStoreSize(field->getType()); + uint64_t fieldSize = TD->getABITypeSize(field->getType()); uint64_t padSize = ((i == e-1 ? Size : cvsLayout->getElementOffset(i+1)) - cvsLayout->getElementOffset(i)) - fieldSize; sizeSoFar += fieldSize + padSize; - // Now print the actual field value without ABI size padding. - EmitGlobalConstant(field, true); + // Now print the actual field value. + EmitGlobalConstant(field); // Insert padding - this may include padding to increase the size of the // current field up to the ABI size (if the struct is not packed) as well @@ -1066,7 +1064,7 @@ void AsmPrinter::EmitGlobalConstant(const Constant *CV, bool Packed) { const VectorType *PTy = CP->getType(); for (unsigned I = 0, E = PTy->getNumElements(); I < E; ++I) - EmitGlobalConstant(CP->getOperand(I), false); + EmitGlobalConstant(CP->getOperand(I)); return; } diff --git a/lib/Target/TargetData.cpp b/lib/Target/TargetData.cpp index 69a8ca902b4..f83adefbee3 100644 --- a/lib/Target/TargetData.cpp +++ b/lib/Target/TargetData.cpp @@ -48,10 +48,7 @@ StructLayout::StructLayout(const StructType *ST, const TargetData &TD) { // Loop over each of the elements, placing them in memory... for (unsigned i = 0, e = NumElements; i != e; ++i) { const Type *Ty = ST->getElementType(i); - unsigned TyAlign = ST->isPacked() ? - 1 : TD.getABITypeAlignment(Ty); - uint64_t TySize = ST->isPacked() ? - TD.getTypeStoreSize(Ty) : TD.getABITypeSize(Ty); + unsigned TyAlign = ST->isPacked() ? 1 : TD.getABITypeAlignment(Ty); // Add padding if necessary to align the data element properly... StructSize = (StructSize + TyAlign - 1)/TyAlign * TyAlign; @@ -60,7 +57,7 @@ StructLayout::StructLayout(const StructType *ST, const TargetData &TD) { StructAlignment = std::max(TyAlign, StructAlignment); MemberOffsets[i] = StructSize; - StructSize += TySize; // Consume space for this data item + StructSize += TD.getABITypeSize(Ty); // Consume space for this data item } // Empty structures have alignment of 1 byte. diff --git a/lib/Transforms/Scalar/ScalarReplAggregates.cpp b/lib/Transforms/Scalar/ScalarReplAggregates.cpp index ee80104f108..2db6db754e6 100644 --- a/lib/Transforms/Scalar/ScalarReplAggregates.cpp +++ b/lib/Transforms/Scalar/ScalarReplAggregates.cpp @@ -737,8 +737,7 @@ void SROA::RewriteBitCastUserOfAlloca(Instruction *BCInst, AllocationInst *AI, /// HasPadding - Return true if the specified type has any structure or /// alignment padding, false otherwise. -static bool HasPadding(const Type *Ty, const TargetData &TD, - bool inPacked = false) { +static bool HasPadding(const Type *Ty, const TargetData &TD) { if (const StructType *STy = dyn_cast(Ty)) { const StructLayout *SL = TD.getStructLayout(STy); unsigned PrevFieldBitOffset = 0; @@ -746,7 +745,7 @@ static bool HasPadding(const Type *Ty, const TargetData &TD, unsigned FieldBitOffset = SL->getElementOffsetInBits(i); // Padding in sub-elements? - if (HasPadding(STy->getElementType(i), TD, STy->isPacked())) + if (HasPadding(STy->getElementType(i), TD)) return true; // Check to see if there is any padding between this element and the @@ -770,12 +769,11 @@ static bool HasPadding(const Type *Ty, const TargetData &TD, } } else if (const ArrayType *ATy = dyn_cast(Ty)) { - return HasPadding(ATy->getElementType(), TD, false); + return HasPadding(ATy->getElementType(), TD); } else if (const VectorType *VTy = dyn_cast(Ty)) { - return HasPadding(VTy->getElementType(), TD, false); + return HasPadding(VTy->getElementType(), TD); } - return inPacked ? - false : TD.getTypeSizeInBits(Ty) != TD.getABITypeSizeInBits(Ty); + return TD.getTypeSizeInBits(Ty) != TD.getABITypeSizeInBits(Ty); } /// isSafeStructAllocaToScalarRepl - Check to see if the specified allocation of diff --git a/test/Other/2008-06-04-FieldSizeInPacked.ll b/test/Other/2008-06-04-FieldSizeInPacked.ll new file mode 100644 index 00000000000..f718dd38e91 --- /dev/null +++ b/test/Other/2008-06-04-FieldSizeInPacked.ll @@ -0,0 +1,14 @@ +; RUN: llvm-as < %s | opt -instcombine | llvm-dis | grep true + +target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128" +target triple = "x86_64-unknown-linux-gnu" + %packed = type <{ x86_fp80, i8 }> + %unpacked = type { x86_fp80, i8 } + +define i1 @q() nounwind { +entry: + %char_p = getelementptr %packed* null, i32 0, i32 1 ; [#uses=1] + %char_u = getelementptr %unpacked* null, i32 0, i32 1 ; [#uses=1] + %res = icmp eq i8* %char_p, %char_u ; [#uses=1] + ret i1 %res +} -- 2.34.1