* [PATCH] rs6000: Fix ICE expanding lxvp and stxvp gimple built-ins [PR101849]
@ 2021-08-10 23:37 Peter Bergner
2021-08-11 16:40 ` Bill Schmidt
2021-08-12 18:20 ` David Edelsohn
0 siblings, 2 replies; 6+ messages in thread
From: Peter Bergner @ 2021-08-10 23:37 UTC (permalink / raw)
To: Segher Boessenkool, David Edelsohn
Cc: GCC Patches, Bill Schmidt, Rajalakshmi Srinivasaraghavan
PR101849 shows we ICE on a test case when we pass a non __vector_pair *
pointer to the __builtin_vsx_lxvp and __builtin_vsx_stxvp built-ins
that is cast to __vector_pair *. The problem is that when we expand
the built-in, the cast has already been removed from gimple and we are
only given the base pointer. The solution used here (which fixes the ICE)
is to catch this case and convert the pointer to a __vector_pair * pointer
when expanding the built-in.
This passed bootstrap and regression testing on powerpc64le-linux with
no regressions. Ok for mainline? This also affects GCC 11 and 10, so
ok there too after it has baked on trunk for a few days?
Peter
gcc/
PR target/101849
* config/rs6000/rs6000-call.c (rs6000_gimple_fold_mma_builtin): Cast
pointer to __vector_pair *.
gcc/testsuite/
PR target/101849
* gcc.target/powerpc/pr101849.c: New test.
diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c
index 904e104c058..d04011c0489 100644
--- a/gcc/config/rs6000/rs6000-call.c
+++ b/gcc/config/rs6000/rs6000-call.c
@@ -11919,6 +11919,9 @@ rs6000_gimple_fold_mma_builtin (gimple_stmt_iterator *gsi)
tree offset = gimple_call_arg (stmt, 0);
tree ptr = gimple_call_arg (stmt, 1);
tree lhs = gimple_call_lhs (stmt);
+ if (TREE_TYPE (TREE_TYPE (ptr)) != vector_pair_type_node)
+ ptr = build1 (VIEW_CONVERT_EXPR,
+ build_pointer_type (vector_pair_type_node), ptr);
tree mem = build_simple_mem_ref (build2 (POINTER_PLUS_EXPR,
TREE_TYPE (ptr), ptr, offset));
gimplify_assign (lhs, mem, &new_seq);
@@ -11932,6 +11935,9 @@ rs6000_gimple_fold_mma_builtin (gimple_stmt_iterator *gsi)
tree src = gimple_call_arg (stmt, 0);
tree offset = gimple_call_arg (stmt, 1);
tree ptr = gimple_call_arg (stmt, 2);
+ if (TREE_TYPE (TREE_TYPE (ptr)) != vector_pair_type_node)
+ ptr = build1 (VIEW_CONVERT_EXPR,
+ build_pointer_type (vector_pair_type_node), ptr);
tree mem = build_simple_mem_ref (build2 (POINTER_PLUS_EXPR,
TREE_TYPE (ptr), ptr, offset));
gimplify_assign (mem, src, &new_seq);
diff --git a/gcc/testsuite/gcc.target/powerpc/pr101849.c b/gcc/testsuite/gcc.target/powerpc/pr101849.c
new file mode 100644
index 00000000000..6d2e3b79282
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr101849.c
@@ -0,0 +1,19 @@
+/* PR target/101849 */
+/* { dg-do compile } */
+/* { dg-require-effective-target power10_ok } */
+/* { dg-options "-O2 -mdejagnu-cpu=power10" } */
+
+/* Verify we do not ICE on the tests below. */
+
+__vector_pair vp;
+void
+foo (double *x)
+{
+ vp = __builtin_vsx_lxvp (0, (__vector_pair *)(void *)x);
+}
+
+void
+bar (__vector_pair *src, double *x)
+{
+ __builtin_vsx_stxvp (*src, 0, (__vector_pair *)(void *)x);
+}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rs6000: Fix ICE expanding lxvp and stxvp gimple built-ins [PR101849]
2021-08-10 23:37 [PATCH] rs6000: Fix ICE expanding lxvp and stxvp gimple built-ins [PR101849] Peter Bergner
@ 2021-08-11 16:40 ` Bill Schmidt
2021-08-12 18:20 ` David Edelsohn
1 sibling, 0 replies; 6+ messages in thread
From: Bill Schmidt @ 2021-08-11 16:40 UTC (permalink / raw)
To: Peter Bergner, Segher Boessenkool, David Edelsohn
Cc: GCC Patches, Rajalakshmi Srinivasaraghavan
Hi Peter,
LGTM. Still needs maintainer review, of course. :)
Bill
On 8/10/21 6:37 PM, Peter Bergner wrote:
> PR101849 shows we ICE on a test case when we pass a non __vector_pair *
> pointer to the __builtin_vsx_lxvp and __builtin_vsx_stxvp built-ins
> that is cast to __vector_pair *. The problem is that when we expand
> the built-in, the cast has already been removed from gimple and we are
> only given the base pointer. The solution used here (which fixes the ICE)
> is to catch this case and convert the pointer to a __vector_pair * pointer
> when expanding the built-in.
>
> This passed bootstrap and regression testing on powerpc64le-linux with
> no regressions. Ok for mainline? This also affects GCC 11 and 10, so
> ok there too after it has baked on trunk for a few days?
>
> Peter
>
>
> gcc/
> PR target/101849
> * config/rs6000/rs6000-call.c (rs6000_gimple_fold_mma_builtin): Cast
> pointer to __vector_pair *.
>
> gcc/testsuite/
> PR target/101849
> * gcc.target/powerpc/pr101849.c: New test.
>
>
> diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c
> index 904e104c058..d04011c0489 100644
> --- a/gcc/config/rs6000/rs6000-call.c
> +++ b/gcc/config/rs6000/rs6000-call.c
> @@ -11919,6 +11919,9 @@ rs6000_gimple_fold_mma_builtin (gimple_stmt_iterator *gsi)
> tree offset = gimple_call_arg (stmt, 0);
> tree ptr = gimple_call_arg (stmt, 1);
> tree lhs = gimple_call_lhs (stmt);
> + if (TREE_TYPE (TREE_TYPE (ptr)) != vector_pair_type_node)
> + ptr = build1 (VIEW_CONVERT_EXPR,
> + build_pointer_type (vector_pair_type_node), ptr);
> tree mem = build_simple_mem_ref (build2 (POINTER_PLUS_EXPR,
> TREE_TYPE (ptr), ptr, offset));
> gimplify_assign (lhs, mem, &new_seq);
> @@ -11932,6 +11935,9 @@ rs6000_gimple_fold_mma_builtin (gimple_stmt_iterator *gsi)
> tree src = gimple_call_arg (stmt, 0);
> tree offset = gimple_call_arg (stmt, 1);
> tree ptr = gimple_call_arg (stmt, 2);
> + if (TREE_TYPE (TREE_TYPE (ptr)) != vector_pair_type_node)
> + ptr = build1 (VIEW_CONVERT_EXPR,
> + build_pointer_type (vector_pair_type_node), ptr);
> tree mem = build_simple_mem_ref (build2 (POINTER_PLUS_EXPR,
> TREE_TYPE (ptr), ptr, offset));
> gimplify_assign (mem, src, &new_seq);
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr101849.c b/gcc/testsuite/gcc.target/powerpc/pr101849.c
> new file mode 100644
> index 00000000000..6d2e3b79282
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr101849.c
> @@ -0,0 +1,19 @@
> +/* PR target/101849 */
> +/* { dg-do compile } */
> +/* { dg-require-effective-target power10_ok } */
> +/* { dg-options "-O2 -mdejagnu-cpu=power10" } */
> +
> +/* Verify we do not ICE on the tests below. */
> +
> +__vector_pair vp;
> +void
> +foo (double *x)
> +{
> + vp = __builtin_vsx_lxvp (0, (__vector_pair *)(void *)x);
> +}
> +
> +void
> +bar (__vector_pair *src, double *x)
> +{
> + __builtin_vsx_stxvp (*src, 0, (__vector_pair *)(void *)x);
> +}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rs6000: Fix ICE expanding lxvp and stxvp gimple built-ins [PR101849]
2021-08-10 23:37 [PATCH] rs6000: Fix ICE expanding lxvp and stxvp gimple built-ins [PR101849] Peter Bergner
2021-08-11 16:40 ` Bill Schmidt
@ 2021-08-12 18:20 ` David Edelsohn
2021-08-13 17:01 ` Peter Bergner
1 sibling, 1 reply; 6+ messages in thread
From: David Edelsohn @ 2021-08-12 18:20 UTC (permalink / raw)
To: Peter Bergner
Cc: Segher Boessenkool, GCC Patches, Bill Schmidt,
Rajalakshmi Srinivasaraghavan
On Tue, Aug 10, 2021 at 7:37 PM Peter Bergner <bergner@linux.ibm.com> wrote:
>
> PR101849 shows we ICE on a test case when we pass a non __vector_pair *
> pointer to the __builtin_vsx_lxvp and __builtin_vsx_stxvp built-ins
> that is cast to __vector_pair *. The problem is that when we expand
> the built-in, the cast has already been removed from gimple and we are
> only given the base pointer. The solution used here (which fixes the ICE)
> is to catch this case and convert the pointer to a __vector_pair * pointer
> when expanding the built-in.
>
> This passed bootstrap and regression testing on powerpc64le-linux with
> no regressions. Ok for mainline? This also affects GCC 11 and 10, so
> ok there too after it has baked on trunk for a few days?
>
> Peter
>
>
> gcc/
> PR target/101849
> * config/rs6000/rs6000-call.c (rs6000_gimple_fold_mma_builtin): Cast
> pointer to __vector_pair *.
>
> gcc/testsuite/
> PR target/101849
> * gcc.target/powerpc/pr101849.c: New test.
Okay.
Thanks, David
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rs6000: Fix ICE expanding lxvp and stxvp gimple built-ins [PR101849]
2021-08-12 18:20 ` David Edelsohn
@ 2021-08-13 17:01 ` Peter Bergner
2021-08-13 17:15 ` Bill Schmidt
0 siblings, 1 reply; 6+ messages in thread
From: Peter Bergner @ 2021-08-13 17:01 UTC (permalink / raw)
To: David Edelsohn
Cc: Segher Boessenkool, GCC Patches, Bill Schmidt,
Rajalakshmi Srinivasaraghavan
On 8/12/21 1:20 PM, David Edelsohn wrote:
> On Tue, Aug 10, 2021 at 7:37 PM Peter Bergner <bergner@linux.ibm.com> wrote:
>> gcc/
>> PR target/101849
>> * config/rs6000/rs6000-call.c (rs6000_gimple_fold_mma_builtin): Cast
>> pointer to __vector_pair *.
>>
>> gcc/testsuite/
>> PR target/101849
>> * gcc.target/powerpc/pr101849.c: New test.
>
> Okay.
>
> Thanks, David
V2:
The previous patch was "ok'd" by David, but in the bugzilla, richi made
a suggestion that using build2, the ptr's type is ignored and we could just
pass in the pointer type we want, which is a little simpler, so that is
what I have done with this version, which I think is better than the
previous one. I have also expanded the test case a little to test both
constant and non-constant offsets.
This version passed bootstrap and regression testing on powerpc64le-linux
with no regressions. Ok for mainline? This also affects GCC 11 and 10,
so ok there too after it has baked on trunk for a few days?
...or do we want to stick with the previous patch?
Peter
rs6000: Fix ICE expanding lxvp and stxvp gimple built-ins [PR101849]
PR101849 shows we ICE on a test case when we pass a non __vector_pair *
pointer to the __builtin_vsx_lxvp and __builtin_vsx_stxvp built-ins
that is cast to __vector_pair *. The problem is that when we expand
the built-in, the cast has already been removed from gimple and we are
only given the base pointer. The solution used here (which fixes the ICE)
is to always use a __vector_pair * pointer when expanding the built-in.
gcc/
PR target/101849
* config/rs6000/rs6000-call.c (rs6000_gimple_fold_mma_builtin): Force
pointer to __vector_pair *.
gcc/testsuite/
PR target/101849
* gcc.target/powerpc/pr101849.c: New test.
diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c
index 904e104c058..90d7171fa46 100644
--- a/gcc/config/rs6000/rs6000-call.c
+++ b/gcc/config/rs6000/rs6000-call.c
@@ -11919,8 +11919,10 @@ rs6000_gimple_fold_mma_builtin (gimple_stmt_iterator *gsi)
tree offset = gimple_call_arg (stmt, 0);
tree ptr = gimple_call_arg (stmt, 1);
tree lhs = gimple_call_lhs (stmt);
- tree mem = build_simple_mem_ref (build2 (POINTER_PLUS_EXPR,
- TREE_TYPE (ptr), ptr, offset));
+ tree ptr_t = build_pointer_type (vector_pair_type_node);
+ tree mem = build2 (MEM_REF, vector_pair_type_node,
+ build2 (POINTER_PLUS_EXPR, ptr_t, ptr, offset),
+ build_int_cst (ptr_t, 0));
gimplify_assign (lhs, mem, &new_seq);
pop_gimplify_context (NULL);
gsi_replace_with_seq (gsi, new_seq, true);
@@ -11932,8 +11934,10 @@ rs6000_gimple_fold_mma_builtin (gimple_stmt_iterator *gsi)
tree src = gimple_call_arg (stmt, 0);
tree offset = gimple_call_arg (stmt, 1);
tree ptr = gimple_call_arg (stmt, 2);
- tree mem = build_simple_mem_ref (build2 (POINTER_PLUS_EXPR,
- TREE_TYPE (ptr), ptr, offset));
+ tree ptr_t = build_pointer_type (vector_pair_type_node);
+ tree mem = build2 (MEM_REF, vector_pair_type_node,
+ build2 (POINTER_PLUS_EXPR, ptr_t, ptr, offset),
+ build_int_cst (ptr_t, 0));
gimplify_assign (mem, src, &new_seq);
pop_gimplify_context (NULL);
gsi_replace_with_seq (gsi, new_seq, true);
diff --git a/gcc/testsuite/gcc.target/powerpc/pr101849.c b/gcc/testsuite/gcc.target/powerpc/pr101849.c
new file mode 100644
index 00000000000..823fbfe9647
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr101849.c
@@ -0,0 +1,22 @@
+/* PR target/101849 */
+/* { dg-do compile } */
+/* { dg-require-effective-target power10_ok } */
+/* { dg-options "-O2 -mdejagnu-cpu=power10" } */
+
+/* Verify we do not ICE on the tests below. */
+
+void
+foo (__vector_pair *dst, double *x, long offset)
+{
+ dst[0] = __builtin_vsx_lxvp (0, (__vector_pair *)(void *)x);
+ dst[1] = __builtin_vsx_lxvp (32, (__vector_pair *)(void *)x);
+ dst[2] = __builtin_vsx_lxvp (offset, (__vector_pair *)(void *)x);
+}
+
+void
+bar (__vector_pair *src, double *x, long offset)
+{
+ __builtin_vsx_stxvp (src[0], 0, (__vector_pair *)(void *)x);
+ __builtin_vsx_stxvp (src[1], 32, (__vector_pair *)(void *)x);
+ __builtin_vsx_stxvp (src[2], offset, (__vector_pair *)(void *)x);
+}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rs6000: Fix ICE expanding lxvp and stxvp gimple built-ins [PR101849]
2021-08-13 17:01 ` Peter Bergner
@ 2021-08-13 17:15 ` Bill Schmidt
2021-08-19 23:36 ` Peter Bergner
0 siblings, 1 reply; 6+ messages in thread
From: Bill Schmidt @ 2021-08-13 17:15 UTC (permalink / raw)
To: Peter Bergner, David Edelsohn
Cc: Segher Boessenkool, GCC Patches, Rajalakshmi Srinivasaraghavan
Hi Peter,
On 8/13/21 12:01 PM, Peter Bergner wrote:
> On 8/12/21 1:20 PM, David Edelsohn wrote:
>> On Tue, Aug 10, 2021 at 7:37 PM Peter Bergner <bergner@linux.ibm.com> wrote:
>>> gcc/
>>> PR target/101849
>>> * config/rs6000/rs6000-call.c (rs6000_gimple_fold_mma_builtin): Cast
>>> pointer to __vector_pair *.
>>>
>>> gcc/testsuite/
>>> PR target/101849
>>> * gcc.target/powerpc/pr101849.c: New test.
>> Okay.
>>
>> Thanks, David
> V2:
> The previous patch was "ok'd" by David, but in the bugzilla, richi made
> a suggestion that using build2, the ptr's type is ignored and we could just
> pass in the pointer type we want, which is a little simpler, so that is
> what I have done with this version, which I think is better than the
> previous one. I have also expanded the test case a little to test both
> constant and non-constant offsets.
>
> This version passed bootstrap and regression testing on powerpc64le-linux
> with no regressions. Ok for mainline? This also affects GCC 11 and 10,
> so ok there too after it has baked on trunk for a few days?
>
> ...or do we want to stick with the previous patch?
>
> Peter
Honestly, I don't see how it matters. So far as I can tell, all you've
done here is hand-inlined what build_simple_mem_ref would do. So I
guess I have a slight preference for your original patch (but with the
new test case, of course). Up to you guys.
Thanks,
Bill
>
>
>
> rs6000: Fix ICE expanding lxvp and stxvp gimple built-ins [PR101849]
>
> PR101849 shows we ICE on a test case when we pass a non __vector_pair *
> pointer to the __builtin_vsx_lxvp and __builtin_vsx_stxvp built-ins
> that is cast to __vector_pair *. The problem is that when we expand
> the built-in, the cast has already been removed from gimple and we are
> only given the base pointer. The solution used here (which fixes the ICE)
> is to always use a __vector_pair * pointer when expanding the built-in.
>
> gcc/
> PR target/101849
> * config/rs6000/rs6000-call.c (rs6000_gimple_fold_mma_builtin): Force
> pointer to __vector_pair *.
>
> gcc/testsuite/
> PR target/101849
> * gcc.target/powerpc/pr101849.c: New test.
>
>
> diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c
> index 904e104c058..90d7171fa46 100644
> --- a/gcc/config/rs6000/rs6000-call.c
> +++ b/gcc/config/rs6000/rs6000-call.c
> @@ -11919,8 +11919,10 @@ rs6000_gimple_fold_mma_builtin (gimple_stmt_iterator *gsi)
> tree offset = gimple_call_arg (stmt, 0);
> tree ptr = gimple_call_arg (stmt, 1);
> tree lhs = gimple_call_lhs (stmt);
> - tree mem = build_simple_mem_ref (build2 (POINTER_PLUS_EXPR,
> - TREE_TYPE (ptr), ptr, offset));
> + tree ptr_t = build_pointer_type (vector_pair_type_node);
> + tree mem = build2 (MEM_REF, vector_pair_type_node,
> + build2 (POINTER_PLUS_EXPR, ptr_t, ptr, offset),
> + build_int_cst (ptr_t, 0));
> gimplify_assign (lhs, mem, &new_seq);
> pop_gimplify_context (NULL);
> gsi_replace_with_seq (gsi, new_seq, true);
> @@ -11932,8 +11934,10 @@ rs6000_gimple_fold_mma_builtin (gimple_stmt_iterator *gsi)
> tree src = gimple_call_arg (stmt, 0);
> tree offset = gimple_call_arg (stmt, 1);
> tree ptr = gimple_call_arg (stmt, 2);
> - tree mem = build_simple_mem_ref (build2 (POINTER_PLUS_EXPR,
> - TREE_TYPE (ptr), ptr, offset));
> + tree ptr_t = build_pointer_type (vector_pair_type_node);
> + tree mem = build2 (MEM_REF, vector_pair_type_node,
> + build2 (POINTER_PLUS_EXPR, ptr_t, ptr, offset),
> + build_int_cst (ptr_t, 0));
> gimplify_assign (mem, src, &new_seq);
> pop_gimplify_context (NULL);
> gsi_replace_with_seq (gsi, new_seq, true);
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr101849.c b/gcc/testsuite/gcc.target/powerpc/pr101849.c
> new file mode 100644
> index 00000000000..823fbfe9647
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr101849.c
> @@ -0,0 +1,22 @@
> +/* PR target/101849 */
> +/* { dg-do compile } */
> +/* { dg-require-effective-target power10_ok } */
> +/* { dg-options "-O2 -mdejagnu-cpu=power10" } */
> +
> +/* Verify we do not ICE on the tests below. */
> +
> +void
> +foo (__vector_pair *dst, double *x, long offset)
> +{
> + dst[0] = __builtin_vsx_lxvp (0, (__vector_pair *)(void *)x);
> + dst[1] = __builtin_vsx_lxvp (32, (__vector_pair *)(void *)x);
> + dst[2] = __builtin_vsx_lxvp (offset, (__vector_pair *)(void *)x);
> +}
> +
> +void
> +bar (__vector_pair *src, double *x, long offset)
> +{
> + __builtin_vsx_stxvp (src[0], 0, (__vector_pair *)(void *)x);
> + __builtin_vsx_stxvp (src[1], 32, (__vector_pair *)(void *)x);
> + __builtin_vsx_stxvp (src[2], offset, (__vector_pair *)(void *)x);
> +}
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rs6000: Fix ICE expanding lxvp and stxvp gimple built-ins [PR101849]
2021-08-13 17:15 ` Bill Schmidt
@ 2021-08-19 23:36 ` Peter Bergner
0 siblings, 0 replies; 6+ messages in thread
From: Peter Bergner @ 2021-08-19 23:36 UTC (permalink / raw)
To: wschmidt, David Edelsohn; +Cc: Segher Boessenkool, GCC Patches
On 8/13/21 12:15 PM, Bill Schmidt wrote:
> Honestly, I don't see how it matters. So far as I can tell, all you've done
> here is hand-inlined what build_simple_mem_ref would do. So I guess I have
> a slight preference for your original patch (but with the new test case,
> of course).
Ok, I ended up pushing the original patch then with the expanded test case.
I'll let this bake on trunk for a bit before back porting. Thanks everyone.
Peter
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-08-19 23:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-10 23:37 [PATCH] rs6000: Fix ICE expanding lxvp and stxvp gimple built-ins [PR101849] Peter Bergner
2021-08-11 16:40 ` Bill Schmidt
2021-08-12 18:20 ` David Edelsohn
2021-08-13 17:01 ` Peter Bergner
2021-08-13 17:15 ` Bill Schmidt
2021-08-19 23:36 ` Peter Bergner
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).