[llvm-commits] [llvm-gcc-4.2] r46958 - in /llvm-gcc-4.2/trunk/gcc: config/i386/llvm-i386-target.h llvm-types.cpp

[Dale Johannesen]

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 at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits