This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]