Alignment of large arrays in x86_64

[Xinliang David Li]

On Thu, May 27, 2010 at 3:16 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Thu, May 27, 2010 at 2:57 AM, Easwaran Raman <eraman@google.com> wrote:
>> The x86-64 ABI requires arrays greater than 16 bytes to be aligned to
>> 16 byte boundaries.  This is mentioned in the comments inside
>> ix86_local_alignment  and  ix86_data_alignment in config/i386/i386.c,
>> but the corresponding code aligns any aggregate type (not just arrays)
>> of size >= 16 *bits* at 16 byte boundaries.  When I fixed this (patch
>> below), it bootstraps ok, but there were  4 test regressions:
>>
>> FAIL: tmpdir-g++.dg-struct-layout-1/t001
>> cp_compat_x_tst.o-cp_compat_y_tst.o execute
>> FAIL: tmpdir-g++.dg-struct-layout-1/t002
>> cp_compat_x_tst.o-cp_compat_y_tst.o execute
>> FAIL: tmpdir-g++.dg-struct-layout-1/t005
>> cp_compat_x_tst.o-cp_compat_y_tst.o execute
>> FAIL: tmpdir-gcc.dg-struct-layout-1/t002
>> c_compat_x_tst.o-c_compat_y_tst.o execute
>>
>> Am I misinterpreting the ABI requirements or is there something wrong
>> with the assumptions in these tests?
>>
>> Thanks,
>> Easwaran
>>
>>
>> --- config/i386/i386.c  2010-05-26 10:55:38.000000000 -0700
>> +++ config/i386/i386.c  2010-05-24 17:22:00.000000000 -0700
>> @@ -20145,11 +20156,11 @@
>>      to 16byte boundary.  */
>>   if (TARGET_64BIT)
>>     {
>> -      if (AGGREGATE_TYPE_P (type)
>> -          && TYPE_SIZE (type)
>> -          && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST
>> -          && (TREE_INT_CST_LOW (TYPE_SIZE (type)) >= 128
>> -              || TREE_INT_CST_HIGH (TYPE_SIZE (type))) && align < 128)
>> +      if (TREE_CODE (type) == ARRAY_TYPE
>> +          && TYPE_SIZE_UNIT (type)
>> +          && TREE_CODE (TYPE_SIZE_UNIT (type)) == INTEGER_CST
>> +          && (TREE_INT_CST_LOW (TYPE_SIZE_UNIT (type)) >= 128
>> +              || TREE_INT_CST_HIGH (TYPE_SIZE_UNIT (type))) && align < 128)
>
> this check was already correct - now you made it check for 128 bytes.

Wrong diff ?

David

>
>>        return 128;
>>     }
>>
>> @@ -20252,13 +20263,13 @@
>>   if (TARGET_64BIT && optimize_function_for_speed_p (cfun)
>>       && TARGET_SSE)
>>     {
>> -      if (AGGREGATE_TYPE_P (type)
>> +      if (TREE_CODE (type) == ARRAY_TYPE
>>           && (TYPE_MAIN_VARIANT (type)
>>               != TYPE_MAIN_VARIANT (va_list_type_node))
>> -          && TYPE_SIZE (type)
>> -          && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST
>> -          && (TREE_INT_CST_LOW (TYPE_SIZE (type)) >= 16
>> -              || TREE_INT_CST_HIGH (TYPE_SIZE (type))) && align < 128)
>> +          && TYPE_SIZE_UNIT (type)
>> +          && TREE_CODE (TYPE_SIZE_UNIT (type)) == INTEGER_CST
>> +          && (TREE_INT_CST_LOW (TYPE_SIZE_UNIT (type)) >= 16
>> +              || TREE_INT_CST_HIGH (TYPE_SIZE_UNIT (type))) && align < 128)
>
> so better just adjust this constant to 128.
>
> I'm not sure about the aggregate thing - we might still want to
> align large structs for speed reasons.
>
> Richard.
>
>>        return 128;
>>     }
>>   if (TREE_CODE (type) == ARRAY_TYPE)
>>
>