Re: [PATCH] Fix incorrect folding of bitfield in a union on big endian target

[Richard Biener <richard dot guenther at gmail dot com>]

On Mon, Aug 11, 2014 at 9:36 AM, Thomas Preud'homme
<thomas.preudhomme@arm.com> wrote:
> In the code dealing with folding of structure and union initialization, there is a
> check that the size of the constructor is the same as the field being read.
> However, in the case of bitfield this test can be wrong because it relies on
> TYPE_SIZE to get the size of the field being read but TYPE_SIZE returns the
> size of the enclosing integer in that case. This patch also check the size
> parameter which contains the actual size of the field being read.
>
> 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.
>
> [1] https://sourceware.org/ml/binutils/2014-08/msg00014.html
>
> No regression were observed on any of the tests. The ChangeLog is as follows:
>
>
> 2014-08-11  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>         * gimple-fold.c (fold_ctor_reference): Don't fold in presence of
>         bitfields, that is when size doesn't match the size of type or the
>         size of the constructor.
>
>
> diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
> index 3dcb576..6270c34 100644
> --- a/gcc/gimple-fold.c
> +++ b/gcc/gimple-fold.c
> @@ -3099,7 +3099,9 @@ fold_ctor_reference (tree type, tree ctor, unsigned HOST_WIDE_INT offset,
>    if (!AGGREGATE_TYPE_P (TREE_TYPE (ctor)) && !offset
>        /* VIEW_CONVERT_EXPR is defined only for matching sizes.  */
>        && operand_equal_p (TYPE_SIZE (type),
> -                         TYPE_SIZE (TREE_TYPE (ctor)), 0))
> +                         TYPE_SIZE (TREE_TYPE (ctor)), 0)
> +      && !compare_tree_int (TYPE_SIZE (type), size)
> +      && !compare_tree_int (TYPE_SIZE (TREE_TYPE (ctor)), size))

That's now extra compares (the operand_equal_p check now does
a check that can be derived transitively).

So - ok with the operand_equal_p cehck removed.

Also see if this can be backported.

Thanks,
Richard.

>      {
>        ret = canonicalize_constructor_val (unshare_expr (ctor), from_decl);
>        ret = fold_unary (VIEW_CONVERT_EXPR, type, ret);
> diff --git a/gcc/testsuite/gcc.c-torture/execute/bitfld-6.c b/gcc/testsuite/gcc.c-torture/execute/bitfld-6.c
> new file mode 100644
> index 0000000..50927dc
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/execute/bitfld-6.c
> @@ -0,0 +1,23 @@
> +union U
> +{
> +  const int a;
> +  unsigned b : 20;
> +};
> +
> +static union U u = { 0x12345678 };
> +
> +/* Constant folding used to fail to account for endianness when folding a
> +   union.  */
> +
> +int
> +main (void)
> +{
> +#ifdef __BYTE_ORDER__
> +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> +  return u.b - 0x45678;
> +#else
> +  return u.b - 0x12345;
> +#endif
> +#endif
> +  return 0;
> +}
>
> Is it ok for trunk?
>
> Best regards,
>
> Thomas
>
>