On 11/08/14 09:53, Richard Earnshaw wrote:
> On 11/08/14 08:49, Thomas Preud'homme wrote:
>> Being 32-bit wide in size, constant pool entries are always filled as 32-bit
>> quantities. This works fine for little endian system but gives some incorrect
>> results for big endian system when the value is *accessed* with a mode
>> smaller
>> than 32-bit in size. Suppose the case of the value 0x1234 that is accessed
>> as an
>> HImode value. It would be output as 0x0 0x0 0x12 0x34 in a constant pool
>> entry
>> and the 16-bit load that would be done would lead to the value 0x0 in
>> register.
>> The approach here is to transform the value so that it is output correctly by
>> shifting the value left so that the highest byte in its mode ends up in the
>> highest byte of the 32-bit value being output.
>>
>> The patch was tested by running the testsuite with three different builds of
>> GCC:
>> 1) bootstrap of GCC on x86_64-linux-gnu
>> 2) arm-none-eabi cross compiler (defaulting to little endian) with testsuite
>> run under qemu emulqting a Cortex M4
>> 3) arm-none-eabi cross compiler (defaulting to big endian, thanks to patch
>> at [1])
>> with testsuite run under qemu emulating a Cortex M3. Due to the processor
>> used,
>> the test constant-minipool was not run as part of the testsuite but manually
>> with
>> -cpu=cortex-r4
>>
>> [1] https://sourceware.org/ml/binutils/2014-08/msg00014.html
>>
>> The ChangeLog is as follows:
>>
>> *** gcc/ChangeLog ***
>>
>> 2014-07-14 Thomas Preud'homme <thomas.preudho...@arm.com>
>>
>> * config/arm/arm.c (dump_minipool): Fix alignment in minipool of
>> values whose size is less than MINIPOOL_FIX_SIZE on big endian target.
>>
>
> I think this is the wrong place for this sort of fix up. HFmode values
> are fixed up in the consttable_4 pattern and it looks wrong to be that
> HImode values are then fixed up in a different place. We should be
> consistent and do all the fix ups in one location.
>
BTW, this is PR59593, please can you cite that in your change log entry.
R.
> R.
>
>
>> *** gcc/testsuite/ChangeLog ***
>>
>> 2014-07-14 Thomas Preud'homme <thomas.preudho...@arm.com>
>>
>> * gcc.target/arm/constant-pool.c: New test.
>>
>>
>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
>> index 0146fe8..e4e0ef4 100644
>> --- a/gcc/config/arm/arm.c
>> +++ b/gcc/config/arm/arm.c
>> @@ -16507,6 +16507,15 @@ dump_minipool (rtx scan)
>> fputc ('\n', dump_file);
>> }
>>
>> + /* On big-endian target, make sure that padding for values whose
>> + mode size is smaller than MINIPOOL_FIX_SIZE comes after. */
>> + if (BYTES_BIG_ENDIAN && CONST_INT_P (mp->value))
>> + {
>> + int byte_disp = mp->fix_size - GET_MODE_SIZE (mp->mode);
>> + HOST_WIDE_INT val = INTVAL (mp->value);
>> + mp->value = GEN_INT (val << (byte_disp * BITS_PER_UNIT));
>> + }
>> +
>> switch (mp->fix_size)
>> {
>> #ifdef HAVE_consttable_1
>> diff --git a/gcc/testsuite/gcc.target/arm/constant-pool.c
>> b/gcc/testsuite/gcc.target/arm/constant-pool.c
>> new file mode 100644
>> index 0000000..46a1534
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/constant-pool.c
>> @@ -0,0 +1,28 @@
>> +/* { dg-do compile } */
>> +/* { dg-require-effective-target arm_arm_ok } */
>> +/* { dg-options "-O1 -marm -mbig-endian" } */
>> +
>> +unsigned short v = 0x5678;
>> +int i;
>> +int j = 0;
>> +int *ptr = &j;
>> +
>> +int
>> +func (void)
>> +{
>> + for (i = 0; i < 1; ++i)
>> + {
>> + *ptr = -1;
>> + v = 0x1234;
>> + }
>> + return v;
>> +}
>> +
>> +int
>> +main (void)
>> +{
>> + func ();
>> + return v - 0x1234;
>> +}
>> +
>> +/* { dg-final { scan-assembler-not ".word 4660" } } */
>>
>>
>> Is this ok for trunk?
>>
>> Best regards,
>>
>> Thomas
>>
>>
>>
>
>
>
