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.
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
>
>
>
