Re: [PATCH] fix arm neon ICE by widening tree_type's precision field

[Richard Guenther <richard.guenther@gmail.com>]

On Mon, Jun 8, 2009 at 11:32 AM, Nathan Froyd<froydnj@codesourcery.com> wrote:
> ARM's NEON support in GCC includes some rather wide types: XImode, the
> biggest, is a 64-byte integer type.  Compiling the simple testcase in
> the patch below, which uses XImode, results in an ICE.
>
> The reason behind this is because XImode is defined as a
> FRACTIONAL_INT_MODE of 511 bits, rather than an INT_MODE of 64 bytes.
> This hack was done because the 'precision' field of tree_type is only 9
> bits wide...and therefore can't store XImode's actual precision of 512
> bits.  So when we ask mode_for_size for a MODE_INT of 512 bits, it
> thinks that the only reasonable choice is BLKmode--since XImode is not
> a MODE_INT mode.  We eventually wind up calling simplify_subreg with the
> input mode of BLKmode, which triggers an ICE.
>
> The fix is to widen the 'precision' field of tree_type to 10 bits.
> Unfortunately, there's not much space in tree_type to widen that field;
> the structure is nicely packed for 32-bit and 64-bit hosts.  The
> approach I've taken in the patch below is to move packed_flag into
> tree_base and rearrange the bitfields in tree_type.  I realize this is
> slightly gross, but I don't see a better way--suggestions welcome.

Ugh.

> Tested with cross to arm-none-eabi.  OK to commit?  (Need middle-end
> approval for the tree.h changes and ARM approval for arm-modes.def.)

Ok.

Thanks,
Richard.

> -Nathan
>
> 2009-06-08  Nathan Froyd  <froydnj@codesourcery.com>
>
>        * tree.h (tree_base): Add packed_flag, moved from...
>        (tree_type): ...here.  Widen precision field to 10 bits and
>        rearrange fields to pack nicely.
>        (TYPE_PACKED): Update for new location of packed_flag.
>        * config/arm/arm-modes.def (XImode): Define as a INT_MODE.
>
> Index: config/arm/arm-modes.def
> ===================================================================
> --- config/arm/arm-modes.def    (revision 148220)
> +++ config/arm/arm-modes.def    (working copy)
> @@ -62,6 +62,4 @@ VECTOR_MODES (FLOAT, 16);     /*       V
>  INT_MODE (EI, 24);
>  INT_MODE (OI, 32);
>  INT_MODE (CI, 48);
> -/* ??? This should actually have 512 bits but the precision only has 9
> -   bits.  */
> -FRACTIONAL_INT_MODE (XI, 511, 64);
> +INT_MODE (XI, 64);
> Index: testsuite/ChangeLog
> ===================================================================
> --- testsuite/ChangeLog (revision 148220)
> +++ testsuite/ChangeLog (working copy)
> @@ -1,3 +1,7 @@
> +2009-06-08  Nathan Froyd  <froydnj@codesourcery.com>
> +
> +       * gcc.target/arm/neon-dse-1.c: New test.
> +
>  2009-06-05  Jakub Jelinek  <jakub@redhat.com>
>
>        PR middle-end/40340
> Index: testsuite/gcc.target/arm/neon-dse-1.c
> ===================================================================
> --- testsuite/gcc.target/arm/neon-dse-1.c       (revision 0)
> +++ testsuite/gcc.target/arm/neon-dse-1.c       (revision 0)
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target arm_neon_ok } */
> +/* { dg-options "-O1" } */
> +/* { dg-add-options arm_neon } */
> +
> +#include <arm_neon.h>
> +
> +void neon_internal_error(int *dst, int *src)
> +{
> +  uint16x8x4_t sval;
> +
> +  sval = vld4q_u16((void *)src);
> +  vst4q_u16((void *)dst,sval);
> +}
> Index: tree.h
> ===================================================================
> --- tree.h      (revision 148220)
> +++ tree.h      (working copy)
> @@ -373,8 +373,10 @@ struct GTY(()) tree_base {
>   unsigned lang_flag_6 : 1;
>
>   unsigned visited : 1;
> +  /* For tree_type.  */
> +  unsigned packed_flag : 1;
>
> -  unsigned spare : 23;
> +  unsigned spare : 22;
>
>   union tree_ann_d *ann;
>  };
> @@ -2219,7 +2221,7 @@ extern enum machine_mode vector_type_mod
>
>  /* Indicated that objects of this type should be laid out in as
>    compact a way as possible.  */
> -#define TYPE_PACKED(NODE) (TYPE_CHECK (NODE)->type.packed_flag)
> +#define TYPE_PACKED(NODE) (TYPE_CHECK (NODE)->common.base.packed_flag)
>
>  /* Used by type_contains_placeholder_p to avoid recomputation.
>    Values are: 0 (unknown), 1 (false), 2 (true).  Never access
> @@ -2237,17 +2239,16 @@ struct GTY(()) tree_type {
>   tree attributes;
>   unsigned int uid;
>
> -  unsigned int precision : 9;
> -  ENUM_BITFIELD(machine_mode) mode : 7;
> -
> -  unsigned string_flag : 1;
> +  unsigned int precision : 10;
>   unsigned no_force_blk_flag : 1;
>   unsigned needs_constructing_flag : 1;
>   unsigned transparent_union_flag : 1;
> -  unsigned packed_flag : 1;
>   unsigned restrict_flag : 1;
>   unsigned contains_placeholder_bits : 2;
>
> +  ENUM_BITFIELD(machine_mode) mode : 7;
> +  unsigned string_flag : 1;
> +
>   unsigned lang_flag_0 : 1;
>   unsigned lang_flag_1 : 1;
>   unsigned lang_flag_2 : 1;
>