* [PATCH] rs6000: Fix an assertion in update_target_cost_per_stmt [PR103702]
@ 2021-12-23 2:06 Kewen.Lin
2022-01-13 1:58 ` PING^1 " Kewen.Lin
0 siblings, 1 reply; 5+ messages in thread
From: Kewen.Lin @ 2021-12-23 2:06 UTC (permalink / raw)
To: GCC Patches; +Cc: Segher Boessenkool, david Edelsohn, Bill Schmidt
Hi,
This patch is to fix one wrong assertion which is too aggressive.
Vectorizer can do vec_construct costing for the vector type which
only has one unit. For the failed case, the passed-in vector type
is "vector(1) int", though it doesn't end up with any construction
eventually. We have to handle this kind of input in function
rs6000_cost_data::update_target_cost_per_stmt.
Bootstrapped and regtested on powerpc64le-linux-gnu P9 and
powerpc64-linux-gnu P8.
Is it ok for trunk?
BR,
Kewen
-----
gcc/ChangeLog:
PR target/103702
* config/rs6000/rs6000.c
(rs6000_cost_data::update_target_cost_per_stmt): Fix one wrong
assertion with early return.
gcc/testsuite/ChangeLog:
PR target/103702
* gcc.target/powerpc/pr103702.c: New test.
---
gcc/config/rs6000/rs6000.c | 7 ++++--
gcc/testsuite/gcc.target/powerpc/pr103702.c | 24 +++++++++++++++++++++
2 files changed, 29 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/powerpc/pr103702.c
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 0b09713b2f5..37f07fe5358 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -5461,8 +5461,11 @@ rs6000_cost_data::update_target_cost_per_stmt (vect_cost_for_stmt kind,
{
tree vectype = STMT_VINFO_VECTYPE (stmt_info);
unsigned int nunits = vect_nunits_for_cost (vectype);
- /* We don't expect strided/elementwise loads for just 1 nunit. */
- gcc_assert (nunits > 1);
+ /* As PR103702 shows, it's possible that vectorizer wants to do
+ costings for only one unit here, it's no need to do any
+ penalization for it, so simply early return here. */
+ if (nunits == 1)
+ return;
/* i386 port adopts nunits * stmt_cost as the penalized cost
for this kind of penalization, we used to follow it but
found it could result in an unreliable body cost especially
diff --git a/gcc/testsuite/gcc.target/powerpc/pr103702.c b/gcc/testsuite/gcc.target/powerpc/pr103702.c
new file mode 100644
index 00000000000..585946fd64b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr103702.c
@@ -0,0 +1,24 @@
+/* We don't have one powerpc.*_ok for Power6, use altivec_ok conservatively. */
+/* { dg-require-effective-target powerpc_altivec_ok } */
+/* { dg-options "-mdejagnu-cpu=power6 -O2 -ftree-loop-vectorize -fno-tree-scev-cprop" } */
+
+/* Verify there is no ICE. */
+
+unsigned short a, e;
+int *b, *d;
+int c;
+extern int fn2 ();
+void
+fn1 ()
+{
+ void *f;
+ for (;;)
+ {
+ fn2 ();
+ b = f;
+ e = 0;
+ for (; e < a; ++e)
+ b[e] = d[e * c];
+ }
+}
+
--
2.27.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* PING^1 [PATCH] rs6000: Fix an assertion in update_target_cost_per_stmt [PR103702]
2021-12-23 2:06 [PATCH] rs6000: Fix an assertion in update_target_cost_per_stmt [PR103702] Kewen.Lin
@ 2022-01-13 1:58 ` Kewen.Lin
2022-01-26 2:14 ` PING^2 " Kewen.Lin
0 siblings, 1 reply; 5+ messages in thread
From: Kewen.Lin @ 2022-01-13 1:58 UTC (permalink / raw)
To: GCC Patches; +Cc: Bill Schmidt, david Edelsohn, Segher Boessenkool
Gentle ping:
https://gcc.gnu.org/pipermail/gcc-patches/2021-December/587309.html
on 2021/12/23 上午10:06, Kewen.Lin via Gcc-patches wrote:
> Hi,
>
> This patch is to fix one wrong assertion which is too aggressive.
> Vectorizer can do vec_construct costing for the vector type which
> only has one unit. For the failed case, the passed-in vector type
> is "vector(1) int", though it doesn't end up with any construction
> eventually. We have to handle this kind of input in function
> rs6000_cost_data::update_target_cost_per_stmt.
>
> Bootstrapped and regtested on powerpc64le-linux-gnu P9 and
> powerpc64-linux-gnu P8.
>
> Is it ok for trunk?
>
> BR,
> Kewen
> -----
> gcc/ChangeLog:
>
> PR target/103702
> * config/rs6000/rs6000.c
> (rs6000_cost_data::update_target_cost_per_stmt): Fix one wrong
> assertion with early return.
>
> gcc/testsuite/ChangeLog:
>
> PR target/103702
> * gcc.target/powerpc/pr103702.c: New test.
> ---
> gcc/config/rs6000/rs6000.c | 7 ++++--
> gcc/testsuite/gcc.target/powerpc/pr103702.c | 24 +++++++++++++++++++++
> 2 files changed, 29 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/powerpc/pr103702.c
>
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index 0b09713b2f5..37f07fe5358 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -5461,8 +5461,11 @@ rs6000_cost_data::update_target_cost_per_stmt (vect_cost_for_stmt kind,
> {
> tree vectype = STMT_VINFO_VECTYPE (stmt_info);
> unsigned int nunits = vect_nunits_for_cost (vectype);
> - /* We don't expect strided/elementwise loads for just 1 nunit. */
> - gcc_assert (nunits > 1);
> + /* As PR103702 shows, it's possible that vectorizer wants to do
> + costings for only one unit here, it's no need to do any
> + penalization for it, so simply early return here. */
> + if (nunits == 1)
> + return;
> /* i386 port adopts nunits * stmt_cost as the penalized cost
> for this kind of penalization, we used to follow it but
> found it could result in an unreliable body cost especially
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr103702.c b/gcc/testsuite/gcc.target/powerpc/pr103702.c
> new file mode 100644
> index 00000000000..585946fd64b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr103702.c
> @@ -0,0 +1,24 @@
> +/* We don't have one powerpc.*_ok for Power6, use altivec_ok conservatively. */
> +/* { dg-require-effective-target powerpc_altivec_ok } */
> +/* { dg-options "-mdejagnu-cpu=power6 -O2 -ftree-loop-vectorize -fno-tree-scev-cprop" } */
> +
> +/* Verify there is no ICE. */
> +
> +unsigned short a, e;
> +int *b, *d;
> +int c;
> +extern int fn2 ();
> +void
> +fn1 ()
> +{
> + void *f;
> + for (;;)
> + {
> + fn2 ();
> + b = f;
> + e = 0;
> + for (; e < a; ++e)
> + b[e] = d[e * c];
> + }
> +}
> +
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* PING^2 [PATCH] rs6000: Fix an assertion in update_target_cost_per_stmt [PR103702]
2022-01-13 1:58 ` PING^1 " Kewen.Lin
@ 2022-01-26 2:14 ` Kewen.Lin
2022-01-26 7:28 ` Richard Biener
0 siblings, 1 reply; 5+ messages in thread
From: Kewen.Lin @ 2022-01-26 2:14 UTC (permalink / raw)
To: GCC Patches; +Cc: Bill Schmidt, Segher Boessenkool, david Edelsohn
Gentle ping:
https://gcc.gnu.org/pipermail/gcc-patches/2021-December/587309.html
BR,
Kewen
> on 2021/12/23 上午10:06, Kewen.Lin via Gcc-patches wrote:
>> Hi,
>>
>> This patch is to fix one wrong assertion which is too aggressive.
>> Vectorizer can do vec_construct costing for the vector type which
>> only has one unit. For the failed case, the passed-in vector type
>> is "vector(1) int", though it doesn't end up with any construction
>> eventually. We have to handle this kind of input in function
>> rs6000_cost_data::update_target_cost_per_stmt.
>>
>> Bootstrapped and regtested on powerpc64le-linux-gnu P9 and
>> powerpc64-linux-gnu P8.
>>
>> Is it ok for trunk?
>>
>> BR,
>> Kewen
>> -----
>> gcc/ChangeLog:
>>
>> PR target/103702
>> * config/rs6000/rs6000.c
>> (rs6000_cost_data::update_target_cost_per_stmt): Fix one wrong
>> assertion with early return.
>>
>> gcc/testsuite/ChangeLog:
>>
>> PR target/103702
>> * gcc.target/powerpc/pr103702.c: New test.
>> ---
>> gcc/config/rs6000/rs6000.c | 7 ++++--
>> gcc/testsuite/gcc.target/powerpc/pr103702.c | 24 +++++++++++++++++++++
>> 2 files changed, 29 insertions(+), 2 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.target/powerpc/pr103702.c
>>
>> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
>> index 0b09713b2f5..37f07fe5358 100644
>> --- a/gcc/config/rs6000/rs6000.c
>> +++ b/gcc/config/rs6000/rs6000.c
>> @@ -5461,8 +5461,11 @@ rs6000_cost_data::update_target_cost_per_stmt (vect_cost_for_stmt kind,
>> {
>> tree vectype = STMT_VINFO_VECTYPE (stmt_info);
>> unsigned int nunits = vect_nunits_for_cost (vectype);
>> - /* We don't expect strided/elementwise loads for just 1 nunit. */
>> - gcc_assert (nunits > 1);
>> + /* As PR103702 shows, it's possible that vectorizer wants to do
>> + costings for only one unit here, it's no need to do any
>> + penalization for it, so simply early return here. */
>> + if (nunits == 1)
>> + return;
>> /* i386 port adopts nunits * stmt_cost as the penalized cost
>> for this kind of penalization, we used to follow it but
>> found it could result in an unreliable body cost especially
>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr103702.c b/gcc/testsuite/gcc.target/powerpc/pr103702.c
>> new file mode 100644
>> index 00000000000..585946fd64b
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/powerpc/pr103702.c
>> @@ -0,0 +1,24 @@
>> +/* We don't have one powerpc.*_ok for Power6, use altivec_ok conservatively. */
>> +/* { dg-require-effective-target powerpc_altivec_ok } */
>> +/* { dg-options "-mdejagnu-cpu=power6 -O2 -ftree-loop-vectorize -fno-tree-scev-cprop" } */
>> +
>> +/* Verify there is no ICE. */
>> +
>> +unsigned short a, e;
>> +int *b, *d;
>> +int c;
>> +extern int fn2 ();
>> +void
>> +fn1 ()
>> +{
>> + void *f;
>> + for (;;)
>> + {
>> + fn2 ();
>> + b = f;
>> + e = 0;
>> + for (; e < a; ++e)
>> + b[e] = d[e * c];
>> + }
>> +}
>> +
>> --
>> 2.27.0
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: PING^2 [PATCH] rs6000: Fix an assertion in update_target_cost_per_stmt [PR103702]
2022-01-26 2:14 ` PING^2 " Kewen.Lin
@ 2022-01-26 7:28 ` Richard Biener
2022-01-27 11:19 ` Kewen.Lin
0 siblings, 1 reply; 5+ messages in thread
From: Richard Biener @ 2022-01-26 7:28 UTC (permalink / raw)
To: Kewen.Lin; +Cc: GCC Patches, Bill Schmidt, david Edelsohn, Segher Boessenkool
On Wed, Jan 26, 2022 at 3:15 AM Kewen.Lin via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Gentle ping:
>
> https://gcc.gnu.org/pipermail/gcc-patches/2021-December/587309.html
OK.
Thanks,
Richard.
> BR,
> Kewen
>
> > on 2021/12/23 上午10:06, Kewen.Lin via Gcc-patches wrote:
> >> Hi,
> >>
> >> This patch is to fix one wrong assertion which is too aggressive.
> >> Vectorizer can do vec_construct costing for the vector type which
> >> only has one unit. For the failed case, the passed-in vector type
> >> is "vector(1) int", though it doesn't end up with any construction
> >> eventually. We have to handle this kind of input in function
> >> rs6000_cost_data::update_target_cost_per_stmt.
> >>
> >> Bootstrapped and regtested on powerpc64le-linux-gnu P9 and
> >> powerpc64-linux-gnu P8.
> >>
> >> Is it ok for trunk?
> >>
> >> BR,
> >> Kewen
> >> -----
> >> gcc/ChangeLog:
> >>
> >> PR target/103702
> >> * config/rs6000/rs6000.c
> >> (rs6000_cost_data::update_target_cost_per_stmt): Fix one wrong
> >> assertion with early return.
> >>
> >> gcc/testsuite/ChangeLog:
> >>
> >> PR target/103702
> >> * gcc.target/powerpc/pr103702.c: New test.
> >> ---
> >> gcc/config/rs6000/rs6000.c | 7 ++++--
> >> gcc/testsuite/gcc.target/powerpc/pr103702.c | 24 +++++++++++++++++++++
> >> 2 files changed, 29 insertions(+), 2 deletions(-)
> >> create mode 100644 gcc/testsuite/gcc.target/powerpc/pr103702.c
> >>
> >> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> >> index 0b09713b2f5..37f07fe5358 100644
> >> --- a/gcc/config/rs6000/rs6000.c
> >> +++ b/gcc/config/rs6000/rs6000.c
> >> @@ -5461,8 +5461,11 @@ rs6000_cost_data::update_target_cost_per_stmt (vect_cost_for_stmt kind,
> >> {
> >> tree vectype = STMT_VINFO_VECTYPE (stmt_info);
> >> unsigned int nunits = vect_nunits_for_cost (vectype);
> >> - /* We don't expect strided/elementwise loads for just 1 nunit. */
> >> - gcc_assert (nunits > 1);
> >> + /* As PR103702 shows, it's possible that vectorizer wants to do
> >> + costings for only one unit here, it's no need to do any
> >> + penalization for it, so simply early return here. */
> >> + if (nunits == 1)
> >> + return;
> >> /* i386 port adopts nunits * stmt_cost as the penalized cost
> >> for this kind of penalization, we used to follow it but
> >> found it could result in an unreliable body cost especially
> >> diff --git a/gcc/testsuite/gcc.target/powerpc/pr103702.c b/gcc/testsuite/gcc.target/powerpc/pr103702.c
> >> new file mode 100644
> >> index 00000000000..585946fd64b
> >> --- /dev/null
> >> +++ b/gcc/testsuite/gcc.target/powerpc/pr103702.c
> >> @@ -0,0 +1,24 @@
> >> +/* We don't have one powerpc.*_ok for Power6, use altivec_ok conservatively. */
> >> +/* { dg-require-effective-target powerpc_altivec_ok } */
> >> +/* { dg-options "-mdejagnu-cpu=power6 -O2 -ftree-loop-vectorize -fno-tree-scev-cprop" } */
> >> +
> >> +/* Verify there is no ICE. */
> >> +
> >> +unsigned short a, e;
> >> +int *b, *d;
> >> +int c;
> >> +extern int fn2 ();
> >> +void
> >> +fn1 ()
> >> +{
> >> + void *f;
> >> + for (;;)
> >> + {
> >> + fn2 ();
> >> + b = f;
> >> + e = 0;
> >> + for (; e < a; ++e)
> >> + b[e] = d[e * c];
> >> + }
> >> +}
> >> +
> >> --
> >> 2.27.0
> >>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: PING^2 [PATCH] rs6000: Fix an assertion in update_target_cost_per_stmt [PR103702]
2022-01-26 7:28 ` Richard Biener
@ 2022-01-27 11:19 ` Kewen.Lin
0 siblings, 0 replies; 5+ messages in thread
From: Kewen.Lin @ 2022-01-27 11:19 UTC (permalink / raw)
To: Richard Biener
Cc: GCC Patches, Bill Schmidt, david Edelsohn, Segher Boessenkool
on 2022/1/26 下午3:28, Richard Biener wrote:
> On Wed, Jan 26, 2022 at 3:15 AM Kewen.Lin via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> Gentle ping:
>>
>> https://gcc.gnu.org/pipermail/gcc-patches/2021-December/587309.html
>
> OK.
>
Thanks Richi! Rebased, re-tested and committed as r12-6891.
BR,
Kewen
> Thanks,
> Richard.
>
>> BR,
>> Kewen
>>
>>> on 2021/12/23 上午10:06, Kewen.Lin via Gcc-patches wrote:
>>>> Hi,
>>>>
>>>> This patch is to fix one wrong assertion which is too aggressive.
>>>> Vectorizer can do vec_construct costing for the vector type which
>>>> only has one unit. For the failed case, the passed-in vector type
>>>> is "vector(1) int", though it doesn't end up with any construction
>>>> eventually. We have to handle this kind of input in function
>>>> rs6000_cost_data::update_target_cost_per_stmt.
>>>>
>>>> Bootstrapped and regtested on powerpc64le-linux-gnu P9 and
>>>> powerpc64-linux-gnu P8.
>>>>
>>>> Is it ok for trunk?
>>>>
>>>> BR,
>>>> Kewen
>>>> -----
>>>> gcc/ChangeLog:
>>>>
>>>> PR target/103702
>>>> * config/rs6000/rs6000.c
>>>> (rs6000_cost_data::update_target_cost_per_stmt): Fix one wrong
>>>> assertion with early return.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>> PR target/103702
>>>> * gcc.target/powerpc/pr103702.c: New test.
>>>> ---
>>>> gcc/config/rs6000/rs6000.c | 7 ++++--
>>>> gcc/testsuite/gcc.target/powerpc/pr103702.c | 24 +++++++++++++++++++++
>>>> 2 files changed, 29 insertions(+), 2 deletions(-)
>>>> create mode 100644 gcc/testsuite/gcc.target/powerpc/pr103702.c
>>>>
>>>> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
>>>> index 0b09713b2f5..37f07fe5358 100644
>>>> --- a/gcc/config/rs6000/rs6000.c
>>>> +++ b/gcc/config/rs6000/rs6000.c
>>>> @@ -5461,8 +5461,11 @@ rs6000_cost_data::update_target_cost_per_stmt (vect_cost_for_stmt kind,
>>>> {
>>>> tree vectype = STMT_VINFO_VECTYPE (stmt_info);
>>>> unsigned int nunits = vect_nunits_for_cost (vectype);
>>>> - /* We don't expect strided/elementwise loads for just 1 nunit. */
>>>> - gcc_assert (nunits > 1);
>>>> + /* As PR103702 shows, it's possible that vectorizer wants to do
>>>> + costings for only one unit here, it's no need to do any
>>>> + penalization for it, so simply early return here. */
>>>> + if (nunits == 1)
>>>> + return;
>>>> /* i386 port adopts nunits * stmt_cost as the penalized cost
>>>> for this kind of penalization, we used to follow it but
>>>> found it could result in an unreliable body cost especially
>>>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr103702.c b/gcc/testsuite/gcc.target/powerpc/pr103702.c
>>>> new file mode 100644
>>>> index 00000000000..585946fd64b
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/powerpc/pr103702.c
>>>> @@ -0,0 +1,24 @@
>>>> +/* We don't have one powerpc.*_ok for Power6, use altivec_ok conservatively. */
>>>> +/* { dg-require-effective-target powerpc_altivec_ok } */
>>>> +/* { dg-options "-mdejagnu-cpu=power6 -O2 -ftree-loop-vectorize -fno-tree-scev-cprop" } */
>>>> +
>>>> +/* Verify there is no ICE. */
>>>> +
>>>> +unsigned short a, e;
>>>> +int *b, *d;
>>>> +int c;
>>>> +extern int fn2 ();
>>>> +void
>>>> +fn1 ()
>>>> +{
>>>> + void *f;
>>>> + for (;;)
>>>> + {
>>>> + fn2 ();
>>>> + b = f;
>>>> + e = 0;
>>>> + for (; e < a; ++e)
>>>> + b[e] = d[e * c];
>>>> + }
>>>> +}
>>>> +
>>>> --
>>>> 2.27.0
>>>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-01-27 11:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-23 2:06 [PATCH] rs6000: Fix an assertion in update_target_cost_per_stmt [PR103702] Kewen.Lin
2022-01-13 1:58 ` PING^1 " Kewen.Lin
2022-01-26 2:14 ` PING^2 " Kewen.Lin
2022-01-26 7:28 ` Richard Biener
2022-01-27 11:19 ` Kewen.Lin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).