nvptx: Fix ICE in nvptx_vector_alignment on gcc.dg/attr-vector_size.c

This patch addresses the ICE in gcc.dg/attr-vector_size.c during make -k check on
nvptx-none.  The actual ICE looks like:

testsuite/gcc.dg/attr-vector_size.c:29:1: internal compiler error: in tree_to_shwi, at tree.c:7321
0xf53bf2 tree_to_shwi(tree_node const*)
        ../../gcc/gcc/tree.c:7321
0xff1969 nvptx_vector_alignment
        ../../gcc/gcc/config/nvptx/nvptx.c:5105^M

The problem is that the caller has ensured that TYPE_SIZE(type) is representable as
an unsigned HOST_WIDE_INT, but nvptx_vector_alignment is accessing it as a
signed HOST_WIDE_INT which overflows in pathological conditions.  Amongst those
pathological conditions is that a TYPE_SIZE of zero can sometimes reach this function,
prior to an error being emitted.  Making sure the result is not less than the mode's
alignment and not greater than BIGGEST_ALIGNMENT fixes the ICEs, and generates 
the expected compile-time error messages.

Tested on --target=nvptx-none, with a "make" and "make check" which results in four
fewer unexpected failures and three more expected passes.
Ok for mainline?


2020-06-29  Roger Sayle  <roger@nextmovesoftware.com>

gcc/ChangeLog:
        * config/nvptx/nvptx.c (nvptx_vector_alignment): Use tree_to_uhwi
        to access TYPE_SIZE (type).  Return at least the mode's alignment.

Thanks,
Roger
--
Roger Sayle
NextMove Software
Cambridge, UK

On 6/29/20 7:19 PM, Roger Sayle wrote:
> This patch addresses the ICE in gcc.dg/attr-vector_size.c during make -k check on
> nvptx-none.  The actual ICE looks like:
> 
> testsuite/gcc.dg/attr-vector_size.c:29:1: internal compiler error: in tree_to_shwi, at tree.c:7321
> 0xf53bf2 tree_to_shwi(tree_node const*)
>         ../../gcc/gcc/tree.c:7321
> 0xff1969 nvptx_vector_alignment
>         ../../gcc/gcc/config/nvptx/nvptx.c:5105^M
> 
> The problem is that the caller has ensured that TYPE_SIZE(type) is representable as
> an unsigned HOST_WIDE_INT, but nvptx_vector_alignment is accessing it as a
> signed HOST_WIDE_INT which overflows in pathological conditions.  Amongst those
> pathological conditions is that a TYPE_SIZE of zero can sometimes reach this function,
> prior to an error being emitted.  Making sure the result is not less than the mode's
> alignment and not greater than BIGGEST_ALIGNMENT fixes the ICEs, and generates 
> the expected compile-time error messages.
> 
> Tested on --target=nvptx-none, with a "make" and "make check" which results in four
> fewer unexpected failures and three more expected passes.
> Ok for mainline?
> 
> 
> 2020-06-29  Roger Sayle  <roger@nextmovesoftware.com>
> 
> gcc/ChangeLog:
>         * config/nvptx/nvptx.c (nvptx_vector_alignment): Use tree_to_uhwi
>         to access TYPE_SIZE (type).  Return at least the mode's alignment.
> 
> Thanks,
> Roger
> --
> Roger Sayle
> NextMove Software
> Cambridge, UK
> 
> diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c
> index e3e84df..bfad91b 100644
> --- a/gcc/config/nvptx/nvptx.c
> +++ b/gcc/config/nvptx/nvptx.c
> @@ -5102,9 +5102,10 @@ static const struct attribute_spec nvptx_attribute_table[] =
>  static HOST_WIDE_INT
>  nvptx_vector_alignment (const_tree type)
>  {
> -  HOST_WIDE_INT align = tree_to_shwi (TYPE_SIZE (type));
> -
> -  return MIN (align, BIGGEST_ALIGNMENT);
> +  unsigned HOST_WIDE_INT align = tree_to_uhwi (TYPE_SIZE (type));

In the default target hook, we test tree_fits_uhwi_p, so I prefer we do
the same.

> +  if (align > BIGGEST_ALIGNMENT)
> +    return BIGGEST_ALIGNMENT;

I prefer using MIN to make code easier to read.

> +  return MAX (align, GET_MODE_ALIGNMENT (TYPE_MODE (type)));
>  }
>  
>  /* Indicate that INSN cannot be duplicated.   */

Also, this is related to a PR, number included in commit log.

As of yet untested updated patch attached.

Thanks,
- Tom

Hi Tom,

Beware of doing something just because the default target hook does it that way.
See https://gcc.gnu.org/pipermail/gcc-patches/2020-June/549128.html
which fixes PR middle-end/90597 by correcting default_vector_alignment to do
it the same way as proposed by this nvptx backend patch.

Many thanks for pointing out that the nvptx problem is PR target/90932.

Roger
--

-----Original Message-----
From: Tom de Vries <tdevries@suse.de> 
Sent: 02 July 2020 13:09
To: Roger Sayle <roger@nextmovesoftware.com>; gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] nvptx: Fix ICE in nvptx_vector_alignment on gcc.dg/attr-vector_size.c

On 6/29/20 7:19 PM, Roger Sayle wrote:
> This patch addresses the ICE in gcc.dg/attr-vector_size.c during make 
> -k check on nvptx-none.  The actual ICE looks like:
> 
> testsuite/gcc.dg/attr-vector_size.c:29:1: internal compiler error: in 
> tree_to_shwi, at tree.c:7321
> 0xf53bf2 tree_to_shwi(tree_node const*)
>         ../../gcc/gcc/tree.c:7321
> 0xff1969 nvptx_vector_alignment
>         ../../gcc/gcc/config/nvptx/nvptx.c:5105^M
> 
> The problem is that the caller has ensured that TYPE_SIZE(type) is 
> representable as an unsigned HOST_WIDE_INT, but nvptx_vector_alignment 
> is accessing it as a signed HOST_WIDE_INT which overflows in 
> pathological conditions.  Amongst those pathological conditions is 
> that a TYPE_SIZE of zero can sometimes reach this function, prior to 
> an error being emitted.  Making sure the result is not less than the 
> mode's alignment and not greater than BIGGEST_ALIGNMENT fixes the ICEs, and generates the expected compile-time error messages.
> 
> Tested on --target=nvptx-none, with a "make" and "make check" which 
> results in four fewer unexpected failures and three more expected passes.
> Ok for mainline?
> 
> 
> 2020-06-29  Roger Sayle  <roger@nextmovesoftware.com>
> 
> gcc/ChangeLog:
>         * config/nvptx/nvptx.c (nvptx_vector_alignment): Use tree_to_uhwi
>         to access TYPE_SIZE (type).  Return at least the mode's alignment.
> 
> Thanks,
> Roger
> --
> Roger Sayle
> NextMove Software
> Cambridge, UK
> 
> diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c index 
> e3e84df..bfad91b 100644
> --- a/gcc/config/nvptx/nvptx.c
> +++ b/gcc/config/nvptx/nvptx.c
> @@ -5102,9 +5102,10 @@ static const struct attribute_spec 
> nvptx_attribute_table[] =  static HOST_WIDE_INT  
> nvptx_vector_alignment (const_tree type)  {
> -  HOST_WIDE_INT align = tree_to_shwi (TYPE_SIZE (type));
> -
> -  return MIN (align, BIGGEST_ALIGNMENT);
> +  unsigned HOST_WIDE_INT align = tree_to_uhwi (TYPE_SIZE (type));

In the default target hook, we test tree_fits_uhwi_p, so I prefer we do the same.

> +  if (align > BIGGEST_ALIGNMENT)
> +    return BIGGEST_ALIGNMENT;

I prefer using MIN to make code easier to read.

> +  return MAX (align, GET_MODE_ALIGNMENT (TYPE_MODE (type)));
>  }
>  
>  /* Indicate that INSN cannot be duplicated.   */

Also, this is related to a PR, number included in commit log.

As of yet untested updated patch attached.

Thanks,
- Tom

On 7/2/20 3:10 PM, Roger Sayle wrote:
> Hi Tom,
> 
> Beware of doing something just because the default target hook does it that way.

Well, what I'm saying is that if the default target hook doesn't assume
tree_fits_uhwi_p (size), the safest solution is to do the same in the
nvptx target hook.

Thanks,
- Tom

> See https://gcc.gnu.org/pipermail/gcc-patches/2020-June/549128.html
> which fixes PR middle-end/90597 by correcting default_vector_alignment to do
> it the same way as proposed by this nvptx backend patch.
> 
> Many thanks for pointing out that the nvptx problem is PR target/90932.
> 
> Roger
> --
> 
> -----Original Message-----
> From: Tom de Vries <tdevries@suse.de> 
> Sent: 02 July 2020 13:09
> To: Roger Sayle <roger@nextmovesoftware.com>; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] nvptx: Fix ICE in nvptx_vector_alignment on gcc.dg/attr-vector_size.c
> 
> On 6/29/20 7:19 PM, Roger Sayle wrote:
>> This patch addresses the ICE in gcc.dg/attr-vector_size.c during make 
>> -k check on nvptx-none.  The actual ICE looks like:
>>
>> testsuite/gcc.dg/attr-vector_size.c:29:1: internal compiler error: in 
>> tree_to_shwi, at tree.c:7321
>> 0xf53bf2 tree_to_shwi(tree_node const*)
>>         ../../gcc/gcc/tree.c:7321
>> 0xff1969 nvptx_vector_alignment
>>         ../../gcc/gcc/config/nvptx/nvptx.c:5105^M
>>
>> The problem is that the caller has ensured that TYPE_SIZE(type) is 
>> representable as an unsigned HOST_WIDE_INT, but nvptx_vector_alignment 
>> is accessing it as a signed HOST_WIDE_INT which overflows in 
>> pathological conditions.  Amongst those pathological conditions is 
>> that a TYPE_SIZE of zero can sometimes reach this function, prior to 
>> an error being emitted.  Making sure the result is not less than the 
>> mode's alignment and not greater than BIGGEST_ALIGNMENT fixes the ICEs, and generates the expected compile-time error messages.
>>
>> Tested on --target=nvptx-none, with a "make" and "make check" which 
>> results in four fewer unexpected failures and three more expected passes.
>> Ok for mainline?
>>
>>
>> 2020-06-29  Roger Sayle  <roger@nextmovesoftware.com>
>>
>> gcc/ChangeLog:
>>         * config/nvptx/nvptx.c (nvptx_vector_alignment): Use tree_to_uhwi
>>         to access TYPE_SIZE (type).  Return at least the mode's alignment.
>>
>> Thanks,
>> Roger
>> --
>> Roger Sayle
>> NextMove Software
>> Cambridge, UK
>>
>> diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c index 
>> e3e84df..bfad91b 100644
>> --- a/gcc/config/nvptx/nvptx.c
>> +++ b/gcc/config/nvptx/nvptx.c
>> @@ -5102,9 +5102,10 @@ static const struct attribute_spec 
>> nvptx_attribute_table[] =  static HOST_WIDE_INT  
>> nvptx_vector_alignment (const_tree type)  {
>> -  HOST_WIDE_INT align = tree_to_shwi (TYPE_SIZE (type));
>> -
>> -  return MIN (align, BIGGEST_ALIGNMENT);
>> +  unsigned HOST_WIDE_INT align = tree_to_uhwi (TYPE_SIZE (type));
> 
> In the default target hook, we test tree_fits_uhwi_p, so I prefer we do the same.
> 
>> +  if (align > BIGGEST_ALIGNMENT)
>> +    return BIGGEST_ALIGNMENT;
> 
> I prefer using MIN to make code easier to read.
> 
>> +  return MAX (align, GET_MODE_ALIGNMENT (TYPE_MODE (type)));
>>  }
>>  
>>  /* Indicate that INSN cannot be duplicated.   */
> 
> Also, this is related to a PR, number included in commit log.
> 
> As of yet untested updated patch attached.
> 
> Thanks,
> - Tom
>