[llvm-commits] [llvm-gcc-4.2] r83229 - /llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp

[Bob Wilson]

On Oct 3, 2009, at 7:53 AM, Duncan Sands <baldrick at free.fr> wrote:

> Hi Bob,
>
>> When copying an aggregate, always copy it element-by-element  
>> instead of using
>> memcpy when the aggregate contains a single element, regardless of  
>> the size.
>> For ARM, we set the size limit low (4 bytes) but the <arm_neon.h>  
>> header
>> uses wrapper structs around NEON vector types, and then uses unions  
>> to
>> convert those structs to and from the underlying vector types.   
>> Those structs
>> and unions should not be expanded to memcpy calls.
>
> is this change for correctness, or an optimization?

Optimization.

>
>>   // If the type is small, copy the elements instead of using a  
>> block copy.
>> -  if (TREE_CODE(TYPE_SIZE(type)) == INTEGER_CST &&
>> -      TREE_INT_CST_LOW(TYPE_SIZE_UNIT(type)) <
>> -          TARGET_LLVM_MIN_BYTES_COPY_BY_MEMCPY) {
>> -    const Type *LLVMTy = ConvertType(type);
>> +  const Type *LLVMTy = ConvertType(type);
>> +  unsigned NumElts = CountAggregateElements(LLVMTy);
>> +  if (NumElts == 1 ||
>> +      (TREE_CODE(TYPE_SIZE(type)) == INTEGER_CST &&
>> +       TREE_INT_CST_LOW(TYPE_SIZE_UNIT(type)) <
>> +       TARGET_LLVM_MIN_BYTES_COPY_BY_MEMCPY)) {
>
> This will break structs with fields at variable offsets: when  
> converting
> to an LLVM struct type, all the fields at variable offsets are  
> skipped.
> So the LLVM struct may have only one field, but in fact there are many
> more in the gcc type.  This is the reason for testing whether the size
> of the type is a constant or not.  So please always check that the  
> type
> has constant size, regardless of the number of elements.

Ok, I will add a check for that.