[llvm-commits] [llvm-gcc-4.2] r51723 - in /llvm-gcc-4.2/trunk/gcc: config/i386/llvm-i386-target.h config/i386/llvm-i386.cpp config/rs6000/llvm-rs6000.cpp llvm-abi.h

Evan Cheng evan.cheng at apple.com
Tue Jun 3 17:14:35 PDT 2008


On Jun 3, 2008, at 3:15 PM, Dale Johannesen wrote:

> On Jun 3, 2008, at 12:23 PM, Dale Johannesen wrote:
>> On Jun 2, 2008, at 11:58 PM, Evan Cheng wrote:
>>
>>> Purely trial and error. llvm-gcc before this patch works. :-)
>>>
>>> Evan
>>
>> Revs 51722 and 51723 both work for me; it was broken after this.
>
> It's 51799, so that is me.

Close enough. :-) Thanks for tracking it down.

Evan

>
>
>> Do we have a way to tell what rev was in effect when the nightly  
>> tester first reported a failure?
>>
>>> On Jun 2, 2008, at 7:35 PM, Dale Johannesen wrote:
>>>
>>>> Not saying you're wrong, but why do you think it's this patch?
>>>>
>>>> On Jun 2, 2008, at 5:35 PM, Evan Cheng wrote:
>>>>
>>>>> Hi Dale,
>>>>>
>>>>> Looks like this patch broke 447.dealII on x86-64. Try this:
>>>>>
>>>>> make ENABLE_OPTIMIZED=1 TEST=nightly TARGET_FLAGS="-m64 - 
>>>>> DSPEC_CPU2000_LP64 -DSPEC_CPU_LP64" TARGET_LLCFLAGS="-relocation- 
>>>>> model=pic -disable-fp-elim" EXTRA_LLI_OPTS="-relocation- 
>>>>> model=pic -disable-fp-elim" clean Output/447.dealII.diff-llc
>>>>>
>>>>> Evan
>>>>>
>>>>> On May 29, 2008, at 6:23 PM, Dale Johannesen wrote:
>>>>>
>>>>>> Author: johannes
>>>>>> Date: Thu May 29 20:23:12 2008
>>>>>> New Revision: 51723
>>>>>>
>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=51723&view=rev
>>>>>> Log:
>>>>>> X86-64 ABI fix.  Revert isSingleElementStructOrArray
>>>>>> change in favor of a more general version which handles
>>>>>> the case where there's more than one element correctly.
>>>>>> Fixes gcc.dg/compat/struct-layout-1.exp/t003
>>>>>> and many more.
>>>>>>
>>>>>>
>>>>>> Modified:
>>>>>> llvm-gcc-4.2/trunk/gcc/config/i386/llvm-i386-target.h
>>>>>> llvm-gcc-4.2/trunk/gcc/config/i386/llvm-i386.cpp
>>>>>> llvm-gcc-4.2/trunk/gcc/config/rs6000/llvm-rs6000.cpp
>>>>>> llvm-gcc-4.2/trunk/gcc/llvm-abi.h
>>>>>>
>>>>>> 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=51723&r1=51722&r2=51723&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 Thu  
>>>>>> May 29 20:23:12 2008
>>>>>> @@ -95,17 +95,18 @@
>>>>>> considered as if they were the type of the data field. */
>>>>>> #ifndef LLVM_SHOULD_RETURN_SELT_STRUCT_AS_SCALAR
>>>>>> #define LLVM_SHOULD_RETURN_SELT_STRUCT_AS_SCALAR(X) \
>>>>>> -  isSingleElementStructOrArray(X, true, false, false)
>>>>>> +  isSingleElementStructOrArray(X, true, false)
>>>>>> #endif
>>>>>>
>>>>>> +extern bool  
>>>>>> llvm_x86_should_pass_aggregate_in_integer_regs(tree, unsigned*);
>>>>>> +
>>>>>> /* LLVM_SHOULD_PASS_AGGREGATE_IN_INTEGER_REGS - Return true if  
>>>>>> this aggregate
>>>>>> value should be passed in integer registers.  This differs from  
>>>>>> the usual
>>>>>> -   handling in that x86-64 passes single-int-element unions as  
>>>>>> the type of the
>>>>>> -   field. */
>>>>>> -#define  
>>>>>> LLVM_SHOULD_PASS_AGGREGATE_IN_INTEGER_REGS(X)                \
>>>>>> -   
>>>>>> (TARGET_64BIT 
>>>>>>  ?                                                    \
>>>>>> -   !isSingleElementStructOrArray((X), true, true,  
>>>>>> true) :            \
>>>>>> -   !isSingleElementStructOrArray((X), false, true, false))
>>>>>> +   handling in that x86-64 passes 128-bit structs and unions  
>>>>>> which only
>>>>>> +   contain data in the first 64 bits, as 64-bit objects.   
>>>>>> (These can be
>>>>>> +   created by abusing __attribute__((aligned)).  */
>>>>>> +#define LLVM_SHOULD_PASS_AGGREGATE_IN_INTEGER_REGS(X,  
>>>>>> Y)             \
>>>>>> +  llvm_x86_should_pass_aggregate_in_integer_regs((X), (Y))
>>>>>>
>>>>>> extern bool llvm_x86_should_pass_vector_in_integer_regs(tree);
>>>>>>
>>>>>>
>>>>>> Modified: llvm-gcc-4.2/trunk/gcc/config/i386/llvm-i386.cpp
>>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm-gcc-4.2/trunk/gcc/config/i386/llvm-i386.cpp?rev=51723&r1=51722&r2=51723&view=diff
>>>>>>
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> =================================================================
>>>>>> --- llvm-gcc-4.2/trunk/gcc/config/i386/llvm-i386.cpp (original)
>>>>>> +++ llvm-gcc-4.2/trunk/gcc/config/i386/llvm-i386.cpp Thu May 29  
>>>>>> 20:23:12 2008
>>>>>> @@ -1288,4 +1288,38 @@
>>>>>> return Loc;
>>>>>> }
>>>>>>
>>>>>> +/// llvm_x86_should_pass_aggregate_in_integer_regs - x86-32 is  
>>>>>> same as the
>>>>>> +/// default.  x86-64 detects the case where a type is 16 bytes  
>>>>>> long but
>>>>>> +/// only 8 of them are passed, the rest being padding (*size  
>>>>>> is set to 8
>>>>>> +/// to identify this case).
>>>>>> +bool llvm_x86_should_pass_aggregate_in_integer_regs(tree type,  
>>>>>> unsigned *size)
>>>>>> +{
>>>>>> +  *size = 0;
>>>>>> +  if (TARGET_64BIT) {
>>>>>> +    enum x86_64_reg_class Class[MAX_CLASSES];
>>>>>> +    enum machine_mode Mode = ix86_getNaturalModeForType(type);
>>>>>> +    int NumClasses = ix86_ClassifyArgument(Mode, type, Class,  
>>>>>> 0);
>>>>>> +    if (NumClasses == 1 && (Class[0] == X86_64_INTEGERSI_CLASS  
>>>>>> ||
>>>>>> +                            Class[0] == X86_64_INTEGER_CLASS)) {
>>>>>> +      /* 8 byte object, one int register */
>>>>>> +      return true;
>>>>>> +    }
>>>>>> +    if (NumClasses == 2 && (Class[0] == X86_64_INTEGERSI_CLASS  
>>>>>> ||
>>>>>> +                            Class[0] == X86_64_INTEGER_CLASS)) {
>>>>>> +      if (Class[1] == X86_64_INTEGERSI_CLASS ||
>>>>>> +          Class[1] == X86_64_INTEGER_CLASS)
>>>>>> +        /* 16 byte object, 2 int registers */
>>>>>> +        return true;
>>>>>> +      if (Class[1] == X86_64_NO_CLASS) {
>>>>>> +        /* 16 byte object, only 1st register has information */
>>>>>> +        *size = 8;
>>>>>> +        return true;
>>>>>> +      }
>>>>>> +    }
>>>>>> +    return false;
>>>>>> +  }
>>>>>> +  else
>>>>>> +    return !isSingleElementStructOrArray(type, false, true);
>>>>>> +}
>>>>>> +
>>>>>> /* LLVM LOCAL end (ENTIRE FILE!)  */
>>>>>>
>>>>>> Modified: llvm-gcc-4.2/trunk/gcc/config/rs6000/llvm-rs6000.cpp
>>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm-gcc-4.2/trunk/gcc/config/rs6000/llvm-rs6000.cpp?rev=51723&r1=51722&r2=51723&view=diff
>>>>>>
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> =================================================================
>>>>>> --- llvm-gcc-4.2/trunk/gcc/config/rs6000/llvm-rs6000.cpp  
>>>>>> (original)
>>>>>> +++ llvm-gcc-4.2/trunk/gcc/config/rs6000/llvm-rs6000.cpp Thu  
>>>>>> May 29 20:23:12 2008
>>>>>> @@ -404,7 +404,7 @@
>>>>>> // some zero-length fields as well, must be passed as the field  
>>>>>> type.
>>>>>> // Note this does not apply to long double.
>>>>>> // This is required for ABI correctness.
>>>>>> -  tree tType = isSingleElementStructOrArray(TreeType, true,  
>>>>>> false, false);
>>>>>> +  tree tType = isSingleElementStructOrArray(TreeType, true,  
>>>>>> false);
>>>>>> if (tType && int_size_in_bytes(tType)==Bytes &&  
>>>>>> TYPE_MODE(tType)!=TFmode &&
>>>>>>  (TREE_CODE(tType)!=VECTOR_TYPE || Bytes==16))
>>>>>> return false;
>>>>>> @@ -437,7 +437,7 @@
>>>>>> // Other single-element structs may be passed this way as well,  
>>>>>> but
>>>>>> // only if the type size matches the element's type size  
>>>>>> (structs that
>>>>>> // violate this can be created with __aligned__).
>>>>>> -  tree tType = isSingleElementStructOrArray(TreeType, true,  
>>>>>> false, false);
>>>>>> +  tree tType = isSingleElementStructOrArray(TreeType, true,  
>>>>>> false);
>>>>>> if (tType && int_size_in_bytes(tType)==SrcSize &&  
>>>>>> TYPE_MODE(tType)!=TFmode &&
>>>>>>  (TREE_CODE(tType)!=VECTOR_TYPE || SrcSize==16)) {
>>>>>> Elts.push_back(ConvertType(tType));
>>>>>>
>>>>>> Modified: llvm-gcc-4.2/trunk/gcc/llvm-abi.h
>>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm-gcc-4.2/trunk/gcc/llvm-abi.h?rev=51723&r1=51722&r2=51723&view=diff
>>>>>>
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> =================================================================
>>>>>> --- llvm-gcc-4.2/trunk/gcc/llvm-abi.h (original)
>>>>>> +++ llvm-gcc-4.2/trunk/gcc/llvm-abi.h Thu May 29 20:23:12 2008
>>>>>> @@ -138,21 +138,17 @@
>>>>>> /// rejectFatBitField, and the single element is a bitfield of  
>>>>>> a type that's
>>>>>> /// bigger than the struct, return null anyway.
>>>>>> static tree isSingleElementStructOrArray(tree type, bool  
>>>>>> ignoreZeroLength,
>>>>>> -                                         bool rejectFatBitfield,
>>>>>> -                                         bool acceptUnions) {
>>>>>> +                                         bool  
>>>>>> rejectFatBitfield) {
>>>>>> // Scalars are good.
>>>>>> if (!isAggregateTreeType(type)) return type;
>>>>>>
>>>>>> tree FoundField = 0;
>>>>>> switch (TREE_CODE(type)) {
>>>>>> case QUAL_UNION_TYPE:
>>>>>> +  case UNION_TYPE:     // Single element unions don't count.
>>>>>> case COMPLEX_TYPE:   // Complex values are like 2-element  
>>>>>> records.
>>>>>> default:
>>>>>> return 0;
>>>>>> -  case UNION_TYPE:     // Single element unions don't count.
>>>>>> -    if (!acceptUnions)
>>>>>> -      return 0;
>>>>>> -    // fall through
>>>>>> case RECORD_TYPE:
>>>>>> // If this record has variable length, reject it.
>>>>>> if (TREE_CODE(TYPE_SIZE(type)) != INTEGER_CST)
>>>>>> @@ -178,15 +174,13 @@
>>>>>>    }
>>>>>>  }
>>>>>> return FoundField ? isSingleElementStructOrArray(FoundField,
>>>>>> -                                                      
>>>>>> ignoreZeroLength, false,
>>>>>> -                                                     false)
>>>>>> +                                                      
>>>>>> ignoreZeroLength, false)
>>>>>>                  : 0;
>>>>>> case ARRAY_TYPE:
>>>>>> const ArrayType *Ty = dyn_cast<ArrayType>(ConvertType(type));
>>>>>> if (!Ty || Ty->getNumElements() != 1)
>>>>>>  return 0;
>>>>>> -    return isSingleElementStructOrArray(TREE_TYPE(type),  
>>>>>> false, false,
>>>>>> -                                        false);
>>>>>> +    return isSingleElementStructOrArray(TREE_TYPE(type),  
>>>>>> false, false);
>>>>>> }
>>>>>> }
>>>>>>
>>>>>> @@ -283,8 +277,8 @@
>>>>>> // single element is a bitfield of a type bigger than the  
>>>>>> struct; the code
>>>>>> // for field-by-field struct passing does not handle this one  
>>>>>> right.
>>>>>> #ifndef LLVM_SHOULD_PASS_AGGREGATE_IN_INTEGER_REGS
>>>>>> -#define LLVM_SHOULD_PASS_AGGREGATE_IN_INTEGER_REGS(X) \
>>>>>> -   !isSingleElementStructOrArray(X, false, true, false)
>>>>>> +#define LLVM_SHOULD_PASS_AGGREGATE_IN_INTEGER_REGS(X, Y) \
>>>>>> +   !isSingleElementStructOrArray((X), false, true)
>>>>>> #endif
>>>>>>
>>>>>> // LLVM_SHOULD_RETURN_SELT_STRUCT_AS_SCALAR - Return a TYPE  
>>>>>> tree if this single
>>>>>> @@ -295,7 +289,7 @@
>>>>>> // by abusing the __aligned__ attribute.)
>>>>>> #ifndef LLVM_SHOULD_RETURN_SELT_STRUCT_AS_SCALAR
>>>>>> #define LLVM_SHOULD_RETURN_SELT_STRUCT_AS_SCALAR(X) \
>>>>>> -  isSingleElementStructOrArray(X, false, false, false)
>>>>>> +  isSingleElementStructOrArray(X, false, false)
>>>>>> #endif
>>>>>>
>>>>>> // LLVM_SHOULD_RETURN_VECTOR_AS_SCALAR - Return a TYPE tree if  
>>>>>> this vector type
>>>>>> @@ -408,6 +402,7 @@
>>>>>> /// their fields.
>>>>>> void HandleArgument(tree type, std::vector<const Type*>  
>>>>>> &ScalarElts,
>>>>>>                  ParameterAttributes *Attributes = NULL) {
>>>>>> +    unsigned Size = 0;
>>>>>> const Type *Ty = ConvertType(type);
>>>>>> // Figure out if this field is zero bits wide, e.g. {} or [0 x  
>>>>>> int].  Do
>>>>>> // not include variable sized fields here.
>>>>>> @@ -418,7 +413,7 @@
>>>>>>  ScalarElts.push_back(PtrTy);
>>>>>> } else if (Ty->getTypeID()==Type::VectorTyID) {
>>>>>>  if (LLVM_SHOULD_PASS_VECTOR_IN_INTEGER_REGS(type)) {
>>>>>> -        PassInIntegerRegisters(type, Ty, ScalarElts);
>>>>>> +        PassInIntegerRegisters(type, Ty, ScalarElts, 0);
>>>>>>  } else {
>>>>>>    C.HandleScalarArgument(Ty, type);
>>>>>>    ScalarElts.push_back(Ty);
>>>>>> @@ -444,8 +439,8 @@
>>>>>>    *Attributes |=
>>>>>>       
>>>>>> ParamAttr::constructAlignmentFromInt(LLVM_BYVAL_ALIGNMENT(type));
>>>>>>  }
>>>>>> -    } else if  
>>>>>> (LLVM_SHOULD_PASS_AGGREGATE_IN_INTEGER_REGS(type)) {
>>>>>> -      PassInIntegerRegisters(type, Ty, ScalarElts);
>>>>>> +    } else if  
>>>>>> (LLVM_SHOULD_PASS_AGGREGATE_IN_INTEGER_REGS(type, &Size)) {
>>>>>> +      PassInIntegerRegisters(type, Ty, ScalarElts, Size);
>>>>>> } else if (isZeroSizedStructOrUnion(type)) {
>>>>>>  // Zero sized struct or union, just drop it!
>>>>>>  ;
>>>>>> @@ -526,10 +521,15 @@
>>>>>>
>>>>>> /// PassInIntegerRegisters - Given an aggregate value that  
>>>>>> should be passed in
>>>>>> /// integer registers, convert it to a structure containing  
>>>>>> ints and pass all
>>>>>> -  /// of the struct elements in.
>>>>>> +  /// of the struct elements in.  If Size is set we pass only  
>>>>>> that many bytes.
>>>>>> void PassInIntegerRegisters(tree type, const Type *Ty,
>>>>>> -                              std::vector<const Type*>  
>>>>>> &ScalarElts) {
>>>>>> -    unsigned Size = TREE_INT_CST_LOW(TYPE_SIZE(type))/8;
>>>>>> +                              std::vector<const Type*>  
>>>>>> &ScalarElts,
>>>>>> +                              unsigned origSize) {
>>>>>> +    unsigned Size;
>>>>>> +    if (origSize)
>>>>>> +      Size = origSize;
>>>>>> +    else
>>>>>> +      Size = TREE_INT_CST_LOW(TYPE_SIZE(type))/8;
>>>>>>
>>>>>> // FIXME: We should preserve all aggregate value alignment  
>>>>>> information.
>>>>>> // Work around to preserve some aggregate value alignment  
>>>>>> information:
>>>>>> @@ -568,7 +568,7 @@
>>>>>>  Elts.push_back(Type::Int8Ty);
>>>>>>  Size -= 1;
>>>>>> }
>>>>>> -    assert(Size == 0 && "Didn't cover value?");
>>>>>> +    assert((origSize || Size == 0) && "Didn't cover value?");
>>>>>> const StructType *STy = StructType::get(Elts, false);
>>>>>>
>>>>>> unsigned i = 0;
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> llvm-commits mailing list
>>>>>> llvm-commits at cs.uiuc.edu
>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>>
>>>>
>>>
>>
>




More information about the llvm-commits mailing list