* [PATCH] Fix handling of VEC_COND_EXPR trap tests [PR100284]
@ 2021-04-27 15:12 Richard Sandiford
2021-04-27 15:15 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Richard Sandiford @ 2021-04-27 15:12 UTC (permalink / raw)
To: gcc-patches; +Cc: rguenther
Now that VEC_COND_EXPR has normal unnested operands,
operation_could_trap_p can treat it like any other expression.
This fixes many testsuite ICEs for SVE, but it turns out that none
of the tests in gcc.target/aarch64/sve were affected. Anyone testing
on non-SVE aarch64 therefore wouldn't have seen it.
Tested on aarch64-linux-gnu (with and without SVE). OK to install?
Richard
gcc/
PR middle-end/100284
* gimple.c (gimple_could_trap_p_1): Remove VEC_COND_EXPR test.
* tree-eh.c (operation_could_trap_p): Handle VEC_COND_EXPR rather
than asserting on it.
gcc/testsuite/
PR middle-end/100284
* gcc.target/aarch64/sve/pr81003.c: New test.
---
gcc/gimple.c | 3 ---
gcc/testsuite/gcc.target/aarch64/sve/pr81003.c | 10 ++++++++++
gcc/tree-eh.c | 6 +++---
3 files changed, 13 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr81003.c
diff --git a/gcc/gimple.c b/gcc/gimple.c
index d067656d315..f1044e9c630 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -2161,9 +2161,6 @@ gimple_could_trap_p_1 (gimple *s, bool include_mem, bool include_stores)
/* For COND_EXPR only the condition may trap. */
if (op == COND_EXPR)
return tree_could_trap_p (gimple_assign_rhs1 (s));
- /* A VEC_COND_EXPR cannot trap. */
- else if (op == VEC_COND_EXPR)
- return false;
/* For comparisons we need to check rhs operand types instead of rhs type
(which is BOOLEAN_TYPE). */
diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
index a68778b9809..601285c401c 100644
--- a/gcc/tree-eh.c
+++ b/gcc/tree-eh.c
@@ -2541,9 +2541,9 @@ operation_could_trap_p (enum tree_code op, bool fp_operation, bool honor_trapv,
bool honor_snans = fp_operation && flag_signaling_nans != 0;
bool handled;
- /* This function cannot tell whether or not COND_EXPR and VEC_COND_EXPR could
- trap, because that depends on the respective condition op. */
- gcc_assert (op != COND_EXPR && op != VEC_COND_EXPR);
+ /* This function cannot tell whether or not COND_EXPR could trap,
+ because that depends on its condition op. */
+ gcc_assert (op != COND_EXPR);
if (TREE_CODE_CLASS (op) != tcc_comparison
&& TREE_CODE_CLASS (op) != tcc_unary
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr81003.c b/gcc/testsuite/gcc.target/aarch64/sve/pr81003.c
new file mode 100644
index 00000000000..661a6f97d6d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/sve/pr81003.c
@@ -0,0 +1,10 @@
+/* { dg-options "-O3" } */
+
+unsigned int a, b;
+
+void
+foo (void)
+{
+ for (b = 0; b < 13; b += 2)
+ a &= !!b;
+}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix handling of VEC_COND_EXPR trap tests [PR100284]
2021-04-27 15:12 [PATCH] Fix handling of VEC_COND_EXPR trap tests [PR100284] Richard Sandiford
@ 2021-04-27 15:15 ` Richard Biener
2021-04-27 15:22 ` Richard Sandiford
0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2021-04-27 15:15 UTC (permalink / raw)
To: Richard Sandiford, gcc-patches
On April 27, 2021 5:12:35 PM GMT+02:00, Richard Sandiford <richard.sandiford@arm.com> wrote:
>Now that VEC_COND_EXPR has normal unnested operands,
>operation_could_trap_p can treat it like any other expression.
>
>This fixes many testsuite ICEs for SVE, but it turns out that none
>of the tests in gcc.target/aarch64/sve were affected. Anyone testing
>on non-SVE aarch64 therefore wouldn't have seen it.
>
>Tested on aarch64-linux-gnu (with and without SVE). OK to install?
Hmm, I now remember why I didn't adjust this. Because on GENERIC the compares are still there and tree_could_trap_p uses the same helper in the end, thus it cannot handle VEC_COND_EXPR this way I think.
Can you double check?
Richard.
>Richard
>
>
>gcc/
> PR middle-end/100284
> * gimple.c (gimple_could_trap_p_1): Remove VEC_COND_EXPR test.
> * tree-eh.c (operation_could_trap_p): Handle VEC_COND_EXPR rather
> than asserting on it.
>
>gcc/testsuite/
> PR middle-end/100284
> * gcc.target/aarch64/sve/pr81003.c: New test.
>---
> gcc/gimple.c | 3 ---
> gcc/testsuite/gcc.target/aarch64/sve/pr81003.c | 10 ++++++++++
> gcc/tree-eh.c | 6 +++---
> 3 files changed, 13 insertions(+), 6 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr81003.c
>
>diff --git a/gcc/gimple.c b/gcc/gimple.c
>index d067656d315..f1044e9c630 100644
>--- a/gcc/gimple.c
>+++ b/gcc/gimple.c
>@@ -2161,9 +2161,6 @@ gimple_could_trap_p_1 (gimple *s, bool
>include_mem, bool include_stores)
> /* For COND_EXPR only the condition may trap. */
> if (op == COND_EXPR)
> return tree_could_trap_p (gimple_assign_rhs1 (s));
>- /* A VEC_COND_EXPR cannot trap. */
>- else if (op == VEC_COND_EXPR)
>- return false;
>
>/* For comparisons we need to check rhs operand types instead of rhs
>type
> (which is BOOLEAN_TYPE). */
>diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
>index a68778b9809..601285c401c 100644
>--- a/gcc/tree-eh.c
>+++ b/gcc/tree-eh.c
>@@ -2541,9 +2541,9 @@ operation_could_trap_p (enum tree_code op, bool
>fp_operation, bool honor_trapv,
> bool honor_snans = fp_operation && flag_signaling_nans != 0;
> bool handled;
>
>- /* This function cannot tell whether or not COND_EXPR and
>VEC_COND_EXPR could
>- trap, because that depends on the respective condition op. */
>- gcc_assert (op != COND_EXPR && op != VEC_COND_EXPR);
>+ /* This function cannot tell whether or not COND_EXPR could trap,
>+ because that depends on its condition op. */
>+ gcc_assert (op != COND_EXPR);
>
> if (TREE_CODE_CLASS (op) != tcc_comparison
> && TREE_CODE_CLASS (op) != tcc_unary
>diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr81003.c
>b/gcc/testsuite/gcc.target/aarch64/sve/pr81003.c
>new file mode 100644
>index 00000000000..661a6f97d6d
>--- /dev/null
>+++ b/gcc/testsuite/gcc.target/aarch64/sve/pr81003.c
>@@ -0,0 +1,10 @@
>+/* { dg-options "-O3" } */
>+
>+unsigned int a, b;
>+
>+void
>+foo (void)
>+{
>+ for (b = 0; b < 13; b += 2)
>+ a &= !!b;
>+}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix handling of VEC_COND_EXPR trap tests [PR100284]
2021-04-27 15:15 ` Richard Biener
@ 2021-04-27 15:22 ` Richard Sandiford
2021-04-27 16:54 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Richard Sandiford @ 2021-04-27 15:22 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
Richard Biener <rguenther@suse.de> writes:
> On April 27, 2021 5:12:35 PM GMT+02:00, Richard Sandiford <richard.sandiford@arm.com> wrote:
>>Now that VEC_COND_EXPR has normal unnested operands,
>>operation_could_trap_p can treat it like any other expression.
>>
>>This fixes many testsuite ICEs for SVE, but it turns out that none
>>of the tests in gcc.target/aarch64/sve were affected. Anyone testing
>>on non-SVE aarch64 therefore wouldn't have seen it.
>>
>>Tested on aarch64-linux-gnu (with and without SVE). OK to install?
>
> Hmm, I now remember why I didn't adjust this. Because on GENERIC the compares are still there and tree_could_trap_p uses the same helper in the end, thus it cannot handle VEC_COND_EXPR this way I think.
But is it meaningful to ask this question about one node of a GENERIC
expression in isolation? I thought you'd need a recursive test.
E.g. an EQ_EXPR could be comparing two integers that are the result
of a potentially-trapping FP operation.
If a caller does ask about only the VEC_COND_EXPR and not its operands,
then false seems like the right answer.
Thanks,
Richard
> Can you double check?
>
> Richard.
>
>>Richard
>>
>>
>>gcc/
>> PR middle-end/100284
>> * gimple.c (gimple_could_trap_p_1): Remove VEC_COND_EXPR test.
>> * tree-eh.c (operation_could_trap_p): Handle VEC_COND_EXPR rather
>> than asserting on it.
>>
>>gcc/testsuite/
>> PR middle-end/100284
>> * gcc.target/aarch64/sve/pr81003.c: New test.
>>---
>> gcc/gimple.c | 3 ---
>> gcc/testsuite/gcc.target/aarch64/sve/pr81003.c | 10 ++++++++++
>> gcc/tree-eh.c | 6 +++---
>> 3 files changed, 13 insertions(+), 6 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr81003.c
>>
>>diff --git a/gcc/gimple.c b/gcc/gimple.c
>>index d067656d315..f1044e9c630 100644
>>--- a/gcc/gimple.c
>>+++ b/gcc/gimple.c
>>@@ -2161,9 +2161,6 @@ gimple_could_trap_p_1 (gimple *s, bool
>>include_mem, bool include_stores)
>> /* For COND_EXPR only the condition may trap. */
>> if (op == COND_EXPR)
>> return tree_could_trap_p (gimple_assign_rhs1 (s));
>>- /* A VEC_COND_EXPR cannot trap. */
>>- else if (op == VEC_COND_EXPR)
>>- return false;
>>
>>/* For comparisons we need to check rhs operand types instead of rhs
>>type
>> (which is BOOLEAN_TYPE). */
>>diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
>>index a68778b9809..601285c401c 100644
>>--- a/gcc/tree-eh.c
>>+++ b/gcc/tree-eh.c
>>@@ -2541,9 +2541,9 @@ operation_could_trap_p (enum tree_code op, bool
>>fp_operation, bool honor_trapv,
>> bool honor_snans = fp_operation && flag_signaling_nans != 0;
>> bool handled;
>>
>>- /* This function cannot tell whether or not COND_EXPR and
>>VEC_COND_EXPR could
>>- trap, because that depends on the respective condition op. */
>>- gcc_assert (op != COND_EXPR && op != VEC_COND_EXPR);
>>+ /* This function cannot tell whether or not COND_EXPR could trap,
>>+ because that depends on its condition op. */
>>+ gcc_assert (op != COND_EXPR);
>>
>> if (TREE_CODE_CLASS (op) != tcc_comparison
>> && TREE_CODE_CLASS (op) != tcc_unary
>>diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr81003.c
>>b/gcc/testsuite/gcc.target/aarch64/sve/pr81003.c
>>new file mode 100644
>>index 00000000000..661a6f97d6d
>>--- /dev/null
>>+++ b/gcc/testsuite/gcc.target/aarch64/sve/pr81003.c
>>@@ -0,0 +1,10 @@
>>+/* { dg-options "-O3" } */
>>+
>>+unsigned int a, b;
>>+
>>+void
>>+foo (void)
>>+{
>>+ for (b = 0; b < 13; b += 2)
>>+ a &= !!b;
>>+}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix handling of VEC_COND_EXPR trap tests [PR100284]
2021-04-27 15:22 ` Richard Sandiford
@ 2021-04-27 16:54 ` Richard Biener
0 siblings, 0 replies; 4+ messages in thread
From: Richard Biener @ 2021-04-27 16:54 UTC (permalink / raw)
To: Richard Sandiford; +Cc: gcc-patches
On April 27, 2021 5:22:56 PM GMT+02:00, Richard Sandiford <richard.sandiford@arm.com> wrote:
>Richard Biener <rguenther@suse.de> writes:
>> On April 27, 2021 5:12:35 PM GMT+02:00, Richard Sandiford
><richard.sandiford@arm.com> wrote:
>>>Now that VEC_COND_EXPR has normal unnested operands,
>>>operation_could_trap_p can treat it like any other expression.
>>>
>>>This fixes many testsuite ICEs for SVE, but it turns out that none
>>>of the tests in gcc.target/aarch64/sve were affected. Anyone testing
>>>on non-SVE aarch64 therefore wouldn't have seen it.
>>>
>>>Tested on aarch64-linux-gnu (with and without SVE). OK to install?
>>
>> Hmm, I now remember why I didn't adjust this. Because on GENERIC the
>compares are still there and tree_could_trap_p uses the same helper in
>the end, thus it cannot handle VEC_COND_EXPR this way I think.
>
>But is it meaningful to ask this question about one node of a GENERIC
>expression in isolation? I thought you'd need a recursive test.
>E.g. an EQ_EXPR could be comparing two integers that are the result
>of a potentially-trapping FP operation.
True.
>If a caller does ask about only the VEC_COND_EXPR and not its operands,
>then false seems like the right answer.
So the patch is OK.
Richard.
>Thanks,
>Richard
>
>> Can you double check?
>>
>> Richard.
>>
>>>Richard
>>>
>>>
>>>gcc/
>>> PR middle-end/100284
>>> * gimple.c (gimple_could_trap_p_1): Remove VEC_COND_EXPR test.
>>> * tree-eh.c (operation_could_trap_p): Handle VEC_COND_EXPR rather
>>> than asserting on it.
>>>
>>>gcc/testsuite/
>>> PR middle-end/100284
>>> * gcc.target/aarch64/sve/pr81003.c: New test.
>>>---
>>> gcc/gimple.c | 3 ---
>>> gcc/testsuite/gcc.target/aarch64/sve/pr81003.c | 10 ++++++++++
>>> gcc/tree-eh.c | 6 +++---
>>> 3 files changed, 13 insertions(+), 6 deletions(-)
>>> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr81003.c
>>>
>>>diff --git a/gcc/gimple.c b/gcc/gimple.c
>>>index d067656d315..f1044e9c630 100644
>>>--- a/gcc/gimple.c
>>>+++ b/gcc/gimple.c
>>>@@ -2161,9 +2161,6 @@ gimple_could_trap_p_1 (gimple *s, bool
>>>include_mem, bool include_stores)
>>> /* For COND_EXPR only the condition may trap. */
>>> if (op == COND_EXPR)
>>> return tree_could_trap_p (gimple_assign_rhs1 (s));
>>>- /* A VEC_COND_EXPR cannot trap. */
>>>- else if (op == VEC_COND_EXPR)
>>>- return false;
>>>
>>>/* For comparisons we need to check rhs operand types instead of rhs
>>>type
>>> (which is BOOLEAN_TYPE). */
>>>diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
>>>index a68778b9809..601285c401c 100644
>>>--- a/gcc/tree-eh.c
>>>+++ b/gcc/tree-eh.c
>>>@@ -2541,9 +2541,9 @@ operation_could_trap_p (enum tree_code op, bool
>>>fp_operation, bool honor_trapv,
>>> bool honor_snans = fp_operation && flag_signaling_nans != 0;
>>> bool handled;
>>>
>>>- /* This function cannot tell whether or not COND_EXPR and
>>>VEC_COND_EXPR could
>>>- trap, because that depends on the respective condition op. */
>>>- gcc_assert (op != COND_EXPR && op != VEC_COND_EXPR);
>>>+ /* This function cannot tell whether or not COND_EXPR could trap,
>>>+ because that depends on its condition op. */
>>>+ gcc_assert (op != COND_EXPR);
>>>
>>> if (TREE_CODE_CLASS (op) != tcc_comparison
>>> && TREE_CODE_CLASS (op) != tcc_unary
>>>diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr81003.c
>>>b/gcc/testsuite/gcc.target/aarch64/sve/pr81003.c
>>>new file mode 100644
>>>index 00000000000..661a6f97d6d
>>>--- /dev/null
>>>+++ b/gcc/testsuite/gcc.target/aarch64/sve/pr81003.c
>>>@@ -0,0 +1,10 @@
>>>+/* { dg-options "-O3" } */
>>>+
>>>+unsigned int a, b;
>>>+
>>>+void
>>>+foo (void)
>>>+{
>>>+ for (b = 0; b < 13; b += 2)
>>>+ a &= !!b;
>>>+}
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-04-27 16:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-27 15:12 [PATCH] Fix handling of VEC_COND_EXPR trap tests [PR100284] Richard Sandiford
2021-04-27 15:15 ` Richard Biener
2021-04-27 15:22 ` Richard Sandiford
2021-04-27 16:54 ` Richard Biener
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).