| Message ID | 20180416161616.28037-1-krebbel@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix ICE with single element vector | expand |
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 >
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 >> >
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 >>> >> >
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);
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(+)