This fixes an ABI issue with unions on i386 (Darwin for sure, and I hope Linux as well). I believe the FIXMEs I added indicate a latent bug, but I'm not sure what the intent was, so could whoever wrote this code originally take a look? (As an aside, assuming that there is a struct type that gets passed the same way as any union type strikes me as dangerous. I don't think there's any reason such a struct needs to exist.)
On Feb 11, 2008, at 10:03 AM, Dale Johannesen wrote: > Author: johannes > Date: Mon Feb 11 12:03:06 2008 > New Revision: 46958 > > URL: http://llvm.org/viewvc/llvm-project?rev=46958&view=rev > Log: > Choose an SSE vector-containing type for the constructed > type that represents a union, if the union contains such > a type. > > > Modified: > llvm-gcc-4.2/trunk/gcc/config/i386/llvm-i386-target.h > llvm-gcc-4.2/trunk/gcc/llvm-types.cpp > > Modified: llvm-gcc-4.2/trunk/gcc/config/i386/llvm-i386-target.h > URL: > http://llvm.org/viewvc/llvm-project/llvm-gcc-4.2/trunk/gcc/config/i386/llvm-i386-target.h?rev=46958&r1=46957&r2=46958&view=diff > > = > = > = > = > = > = > = > = > ====================================================================== > --- llvm-gcc-4.2/trunk/gcc/config/i386/llvm-i386-target.h (original) > +++ llvm-gcc-4.2/trunk/gcc/config/i386/llvm-i386-target.h Mon Feb 11 > 12:03:06 2008 > @@ -83,6 +83,15 @@ > } \ > } > > +/* Aggregates containing SSE vectors are aligned at 16 bytes as > parameters; > + while long double has GCC alignment of 16 bytes (correct for > struct layout) > + but is only 4 byte aligned as a parameter. So if a union type > contains an > + SSE vector, use that as the basis for the constructed LLVM > struct. */ > +#define TARGET_LLVM_COMPARE_UNION_FIELDS(curType, newType, > curAlign, newAlign) \ > + (newAlign==curAlign && TARGET_SSE > && \ > + TheTarget->getTargetLowering()->getByValTypeAlignment(newType) > > \ > + TheTarget->getTargetLowering()->getByValTypeAlignment(curType)) > + > #ifdef LLVM_ABI_H > extern bool llvm_x86_should_pass_aggregate_in_memory(tree, const > Type *); > > > Modified: llvm-gcc-4.2/trunk/gcc/llvm-types.cpp > URL: > http://llvm.org/viewvc/llvm-project/llvm-gcc-4.2/trunk/gcc/llvm-types.cpp?rev=46958&r1=46957&r2=46958&view=diff > > = > = > = > = > = > = > = > = > ====================================================================== > --- llvm-gcc-4.2/trunk/gcc/llvm-types.cpp (original) > +++ llvm-gcc-4.2/trunk/gcc/llvm-types.cpp Mon Feb 11 12:03:06 2008 > @@ -33,6 +33,7 @@ > #include "llvm/TypeSymbolTable.h" > #include "llvm/Target/TargetData.h" > #include "llvm/Target/TargetMachine.h" > +#include "llvm/Target/TargetLowering.h" > #include "llvm/Assembly/Writer.h" > #include "llvm/ADT/DenseMap.h" > #include "llvm/ADT/StringExtras.h" > @@ -1137,7 +1138,7 @@ > // 'sret' functions cannot be 'readnone' or 'readonly'. > if (ABIConverter.isStructReturn()) > RAttributes &= ~(ParamAttr::ReadNone|ParamAttr::ReadOnly); > - > + > // Compute whether the result needs to be zext or sext'd. > RAttributes |= HandleArgumentExtension(TREE_TYPE(type)); > > @@ -2124,6 +2125,8 @@ > > /// ConvertUNION - We know that 'type' is a UNION_TYPE or a > QUAL_UNION_TYPE: > /// convert it to an LLVM type. > +/// This involves creating a struct with the right size and > alignment. In > +/// some cases this is target dependent. > const Type *TypeConverter::ConvertUNION(tree type, tree orig_type) { > if (const Type *Ty = GET_TYPE_LLVM(type)) { > // If we already compiled this type, and if it was not a forward > @@ -2170,17 +2173,25 @@ > // Select TheTy as union type if it meets one of the following > criteria > // 1) UnionTy is 0 > // 2) TheTy alignment is more then UnionTy > - // 3) TheTy size is greater than UnionTy size and TheTy > alignment is equal to UnionTy > + // 3) TheTy size is greater than UnionTy size and TheTy > alignment is equal > + // to UnionTy > // 4) TheTy size is greater then UnionTy size and TheTy is packed > + // FIXME there is no check for packed? > bool useTheTy = false; > if (UnionTy == 0) > useTheTy = true; > else if (Align > MaxAlign) > useTheTy = true; > +#ifdef TARGET_LLVM_COMPARE_UNION_FIELDS > + else > + useTheTy = TARGET_LLVM_COMPARE_UNION_FIELDS(UnionTy, TheTy, > + Align, MaxAlign); > +#else > else if (MaxAlign == Align && Size > MaxSize) > useTheTy = true; > - else if (Size > MaxSize) > + else if (Size > MaxSize) // FIXME really? Seems wrong to > lower alignment > useTheTy = true; > +#endif > > if (useTheTy) { > UnionTy = TheTy; > > > _______________________________________________ > llvm-commits mailing list > llvm-commits@cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits _______________________________________________ llvm-commits mailing list llvm-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
