Fix ICE with single element vector

Message ID 20180416161616.28037-1-krebbel@linux.vnet.ibm.com
State New
Headers show
Series
  • Fix ICE with single element vector
Related show

Commit Message

Andreas Krebbel April 16, 2018, 4:16 p.m.
I did run into an ICE with a single element vector triggered by
dividing the number of elements by 2 with exact_div here:

tree-vect-data-refs.c:5132

      else
	{
	  /* If length is not equal to 3 then only power of 2 is supported.  */
	  gcc_assert (pow2p_hwi (count));
	  poly_uint64 nelt = GET_MODE_NUNITS (mode);

	  /* The encoding has 2 interleaved stepped patterns.  */
	  vec_perm_builder sel (nelt, 2, 3);
	  sel.quick_grow (6);
	  for (i = 0; i < 3; i++)
	    {
	      sel[i * 2] = i;
	      sel[i * 2 + 1] = i + nelt;
	    }
	  vec_perm_indices indices (sel, 2, nelt);
	  if (can_vec_perm_const_p (mode, indices))
	    {
	      for (i = 0; i < 6; i++)
		sel[i] += exact_div (nelt, 2);    <-----
	      indices.new_vector (sel, 2, nelt);
	      if (can_vec_perm_const_p (mode, indices))
		return true;
	    }
	}

The patch adds a check to prevent this.

Ok?

-Andreas-

gcc/ChangeLog:

2018-04-16  Andreas Krebbel  <krebbel@linux.ibm.com>

	* tree-vect-data-refs.c (vect_grouped_store_supported): Exit for
	single element vectors.
---
 gcc/tree-vect-data-refs.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Richard Biener April 18, 2018, 8:25 a.m. | #1
On Mon, Apr 16, 2018 at 6:16 PM, Andreas Krebbel
<krebbel@linux.vnet.ibm.com> wrote:
> I did run into an ICE with a single element vector triggered by
> dividing the number of elements by 2 with exact_div here:
>
> tree-vect-data-refs.c:5132
>
>       else
>         {
>           /* If length is not equal to 3 then only power of 2 is supported.  */
>           gcc_assert (pow2p_hwi (count));
>           poly_uint64 nelt = GET_MODE_NUNITS (mode);
>
>           /* The encoding has 2 interleaved stepped patterns.  */
>           vec_perm_builder sel (nelt, 2, 3);
>           sel.quick_grow (6);
>           for (i = 0; i < 3; i++)
>             {
>               sel[i * 2] = i;
>               sel[i * 2 + 1] = i + nelt;
>             }
>           vec_perm_indices indices (sel, 2, nelt);
>           if (can_vec_perm_const_p (mode, indices))
>             {
>               for (i = 0; i < 6; i++)
>                 sel[i] += exact_div (nelt, 2);    <-----
>               indices.new_vector (sel, 2, nelt);
>               if (can_vec_perm_const_p (mode, indices))
>                 return true;
>             }
>         }
>
> The patch adds a check to prevent this.

Testcase?

What's the group size?

Did it work before the poly-int stuff?  Single-element vectors are
somewhat "special" - where do they appear for you?

Richard.

> Ok?
>
> -Andreas-
>
> gcc/ChangeLog:
>
> 2018-04-16  Andreas Krebbel  <krebbel@linux.ibm.com>
>
>         * tree-vect-data-refs.c (vect_grouped_store_supported): Exit for
>         single element vectors.
> ---
>  gcc/tree-vect-data-refs.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
> index 161a886..01e28ca 100644
> --- a/gcc/tree-vect-data-refs.c
> +++ b/gcc/tree-vect-data-refs.c
> @@ -5135,6 +5135,9 @@ vect_grouped_store_supported (tree vectype, unsigned HOST_WIDE_INT count)
>           gcc_assert (pow2p_hwi (count));
>           poly_uint64 nelt = GET_MODE_NUNITS (mode);
>
> +         if (maybe_eq (nelt, 1U))
> +           return false;
> +
>           /* The encoding has 2 interleaved stepped patterns.  */
>           vec_perm_builder sel (nelt, 2, 3);
>           sel.quick_grow (6);
> --
> 2.9.1
>
Andreas Krebbel April 20, 2018, 9:05 a.m. | #2
On 04/18/2018 10:25 AM, Richard Biener wrote:
> On Mon, Apr 16, 2018 at 6:16 PM, Andreas Krebbel
> <krebbel@linux.vnet.ibm.com> wrote:
>> I did run into an ICE with a single element vector triggered by
>> dividing the number of elements by 2 with exact_div here:
>>
>> tree-vect-data-refs.c:5132
>>
>>       else
>>         {
>>           /* If length is not equal to 3 then only power of 2 is supported.  */
>>           gcc_assert (pow2p_hwi (count));
>>           poly_uint64 nelt = GET_MODE_NUNITS (mode);
>>
>>           /* The encoding has 2 interleaved stepped patterns.  */
>>           vec_perm_builder sel (nelt, 2, 3);
>>           sel.quick_grow (6);
>>           for (i = 0; i < 3; i++)
>>             {
>>               sel[i * 2] = i;
>>               sel[i * 2 + 1] = i + nelt;
>>             }
>>           vec_perm_indices indices (sel, 2, nelt);
>>           if (can_vec_perm_const_p (mode, indices))
>>             {
>>               for (i = 0; i < 6; i++)
>>                 sel[i] += exact_div (nelt, 2);    <-----
>>               indices.new_vector (sel, 2, nelt);
>>               if (can_vec_perm_const_p (mode, indices))
>>                 return true;
>>             }
>>         }
>>
>> The patch adds a check to prevent this.
> 
> Testcase?

I had some trouble extracting a testcase since it ran into another ICE pointing towards a different
problem. But I have one now and will attach it to the BZ.

> What's the group size?
2

> Did it work before the poly-int stuff?  Single-element vectors are
> somewhat "special" - where do they appear for you?

No. The testcase ICEd before poly-int as well. It triggered an ICE in LRA. See BZ.

We use single element vectors for long double data types. We used to have long double hardware
support before getting vector instructions. The long double values were kept in floating point
register pairs and have been passed in memory for historical reasons. We wanted to changed that with
z14 (arch12) where we got instructions which can handle long doubles residing in single vector
registers. Since we could not change the ABI for the existing long double type we rely on V1TF for
that purpose.

I've opened BZ85478 to collect the infos.

-Andreas-

> 
> Richard.
> 
>> Ok?
>>
>> -Andreas-
>>
>> gcc/ChangeLog:
>>
>> 2018-04-16  Andreas Krebbel  <krebbel@linux.ibm.com>
>>
>>         * tree-vect-data-refs.c (vect_grouped_store_supported): Exit for
>>         single element vectors.
>> ---
>>  gcc/tree-vect-data-refs.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
>> index 161a886..01e28ca 100644
>> --- a/gcc/tree-vect-data-refs.c
>> +++ b/gcc/tree-vect-data-refs.c
>> @@ -5135,6 +5135,9 @@ vect_grouped_store_supported (tree vectype, unsigned HOST_WIDE_INT count)
>>           gcc_assert (pow2p_hwi (count));
>>           poly_uint64 nelt = GET_MODE_NUNITS (mode);
>>
>> +         if (maybe_eq (nelt, 1U))
>> +           return false;
>> +
>>           /* The encoding has 2 interleaved stepped patterns.  */
>>           vec_perm_builder sel (nelt, 2, 3);
>>           sel.quick_grow (6);
>> --
>> 2.9.1
>>
>
Richard Biener April 20, 2018, 9:28 a.m. | #3
On Fri, Apr 20, 2018 at 11:05 AM, Andreas Krebbel <krebbel@linux.ibm.com> wrote:
> On 04/18/2018 10:25 AM, Richard Biener wrote:
>> On Mon, Apr 16, 2018 at 6:16 PM, Andreas Krebbel
>> <krebbel@linux.vnet.ibm.com> wrote:
>>> I did run into an ICE with a single element vector triggered by
>>> dividing the number of elements by 2 with exact_div here:
>>>
>>> tree-vect-data-refs.c:5132
>>>
>>>       else
>>>         {
>>>           /* If length is not equal to 3 then only power of 2 is supported.  */
>>>           gcc_assert (pow2p_hwi (count));
>>>           poly_uint64 nelt = GET_MODE_NUNITS (mode);
>>>
>>>           /* The encoding has 2 interleaved stepped patterns.  */
>>>           vec_perm_builder sel (nelt, 2, 3);
>>>           sel.quick_grow (6);
>>>           for (i = 0; i < 3; i++)
>>>             {
>>>               sel[i * 2] = i;
>>>               sel[i * 2 + 1] = i + nelt;
>>>             }
>>>           vec_perm_indices indices (sel, 2, nelt);
>>>           if (can_vec_perm_const_p (mode, indices))
>>>             {
>>>               for (i = 0; i < 6; i++)
>>>                 sel[i] += exact_div (nelt, 2);    <-----
>>>               indices.new_vector (sel, 2, nelt);
>>>               if (can_vec_perm_const_p (mode, indices))
>>>                 return true;
>>>             }
>>>         }
>>>
>>> The patch adds a check to prevent this.
>>
>> Testcase?
>
> I had some trouble extracting a testcase since it ran into another ICE pointing towards a different
> problem. But I have one now and will attach it to the BZ.
>
>> What's the group size?
> 2
>
>> Did it work before the poly-int stuff?  Single-element vectors are
>> somewhat "special" - where do they appear for you?
>
> No. The testcase ICEd before poly-int as well. It triggered an ICE in LRA. See BZ.
>
> We use single element vectors for long double data types. We used to have long double hardware
> support before getting vector instructions. The long double values were kept in floating point
> register pairs and have been passed in memory for historical reasons. We wanted to changed that with
> z14 (arch12) where we got instructions which can handle long doubles residing in single vector
> registers. Since we could not change the ABI for the existing long double type we rely on V1TF for
> that purpose.

I see.  I remember patches from last year from James(?) running into
various issues with
the backend exposing V1mode vectors and actually wanting to allow them
for vectorization.
We now do that which might cause these kind of problems.

> I've opened BZ85478 to collect the infos.

I'll try to have a look.

Richard.

>
> -Andreas-
>
>>
>> Richard.
>>
>>> Ok?
>>>
>>> -Andreas-
>>>
>>> gcc/ChangeLog:
>>>
>>> 2018-04-16  Andreas Krebbel  <krebbel@linux.ibm.com>
>>>
>>>         * tree-vect-data-refs.c (vect_grouped_store_supported): Exit for
>>>         single element vectors.
>>> ---
>>>  gcc/tree-vect-data-refs.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
>>> index 161a886..01e28ca 100644
>>> --- a/gcc/tree-vect-data-refs.c
>>> +++ b/gcc/tree-vect-data-refs.c
>>> @@ -5135,6 +5135,9 @@ vect_grouped_store_supported (tree vectype, unsigned HOST_WIDE_INT count)
>>>           gcc_assert (pow2p_hwi (count));
>>>           poly_uint64 nelt = GET_MODE_NUNITS (mode);
>>>
>>> +         if (maybe_eq (nelt, 1U))
>>> +           return false;
>>> +
>>>           /* The encoding has 2 interleaved stepped patterns.  */
>>>           vec_perm_builder sel (nelt, 2, 3);
>>>           sel.quick_grow (6);
>>> --
>>> 2.9.1
>>>
>>
>

Patch

diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
index 161a886..01e28ca 100644
--- a/gcc/tree-vect-data-refs.c
+++ b/gcc/tree-vect-data-refs.c
@@ -5135,6 +5135,9 @@  vect_grouped_store_supported (tree vectype, unsigned HOST_WIDE_INT count)
 	  gcc_assert (pow2p_hwi (count));
 	  poly_uint64 nelt = GET_MODE_NUNITS (mode);
 
+	  if (maybe_eq (nelt, 1U))
+	    return false;
+
 	  /* The encoding has 2 interleaved stepped patterns.  */
 	  vec_perm_builder sel (nelt, 2, 3);
 	  sel.quick_grow (6);