* PR97849: aarch64: ICE (segfault) during GIMPLE pass: ifcvt
@ 2020-11-18 11:34 Prathamesh Kulkarni
2020-11-18 13:43 ` Richard Biener
0 siblings, 1 reply; 3+ messages in thread
From: Prathamesh Kulkarni @ 2020-11-18 11:34 UTC (permalink / raw)
To: gcc Patches, Richard Biener
[-- Attachment #1: Type: text/plain, Size: 1094 bytes --]
Hi,
For the following test-case (slightly reduced from PR)
int a, b, c;
int g() {
char i = 0;
for (c = 0; c <= 8; c++)
--i;
while (b) {
_Bool f = i <= 0;
a = (a == 0) ? 0 : f / a;
}
}
The compiler segfaults with -O1 -march=armv8.2-a+sve in ifcvt_local_dce.
IIUC, the issue here is that tree-if-conv.c:predicate_rhs_code
processes the following statement:
iftmp.2_7 = a_lsm.10_11 != 0 ? iftmp.2_13 : 0;
and records <iftmp.2_7, iftmp.2_13> mapping.
However RPO VN eliminates iftmp.2_13:
Removing dead stmt iftmp.2_13 = .COND_DIV (_29, _4, a_lsm.10_11, 0);
and we end up replacing iftmp.2_7 with a dead ssa_name in ifcvt_local_dce:
FOR_EACH_VEC_ELT (redundant_ssa_names, i, name_pair)
replace_uses_by (name_pair->first, name_pair->second);
redundant_ssa_names.release ();
resulting in incorrect IR, and segfault down the line.
To avoid clashing of RPO VN with redunant_ssa_names, the patch simply moves
ifcvt_local_dce before do_rpo_vn, which avoids the segfault.
Does that look OK ?
(Altho I guess, doing DCE after VN is better in principle)
Thanks,
Prathamesh
[-- Attachment #2: pr97849-1.diff --]
[-- Type: application/octet-stream, Size: 1169 bytes --]
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr97849.c b/gcc/testsuite/gcc.dg/tree-ssa/pr97849.c
new file mode 100644
index 00000000000..48ec4db7b30
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr97849.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target aarch64_sve } */
+/* { dg-options "-O1 -ftree-vectorize -march=armv8.2-a+sve" } */
+
+int a, b, c;
+
+int g() {
+ char i = 0;
+ for (c = 0; c <= 8; c++)
+ --i;
+
+ while (b) {
+ _Bool f = i <= 0;
+ a = (a == 0) ? 0 : f / a;
+ }
+}
diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c
index 2062758f40f..efc543829f2 100644
--- a/gcc/tree-if-conv.c
+++ b/gcc/tree-if-conv.c
@@ -3124,10 +3124,11 @@ tree_if_conversion (class loop *loop, vec<gimple *> *preds)
exit_bbs = BITMAP_ALLOC (NULL);
bitmap_set_bit (exit_bbs, single_exit (loop)->dest->index);
bitmap_set_bit (exit_bbs, loop->latch->index);
- todo |= do_rpo_vn (cfun, loop_preheader_edge (loop), exit_bbs);
/* Delete dead predicate computations. */
ifcvt_local_dce (loop);
+
+ todo |= do_rpo_vn (cfun, loop_preheader_edge (loop), exit_bbs);
BITMAP_FREE (exit_bbs);
todo |= TODO_cleanup_cfg;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: PR97849: aarch64: ICE (segfault) during GIMPLE pass: ifcvt
2020-11-18 11:34 PR97849: aarch64: ICE (segfault) during GIMPLE pass: ifcvt Prathamesh Kulkarni
@ 2020-11-18 13:43 ` Richard Biener
2020-11-24 1:32 ` Prathamesh Kulkarni
0 siblings, 1 reply; 3+ messages in thread
From: Richard Biener @ 2020-11-18 13:43 UTC (permalink / raw)
To: Prathamesh Kulkarni; +Cc: gcc Patches
On Wed, 18 Nov 2020, Prathamesh Kulkarni wrote:
> Hi,
> For the following test-case (slightly reduced from PR)
> int a, b, c;
>
> int g() {
> char i = 0;
> for (c = 0; c <= 8; c++)
> --i;
>
> while (b) {
> _Bool f = i <= 0;
> a = (a == 0) ? 0 : f / a;
> }
> }
>
> The compiler segfaults with -O1 -march=armv8.2-a+sve in ifcvt_local_dce.
>
> IIUC, the issue here is that tree-if-conv.c:predicate_rhs_code
> processes the following statement:
> iftmp.2_7 = a_lsm.10_11 != 0 ? iftmp.2_13 : 0;
> and records <iftmp.2_7, iftmp.2_13> mapping.
>
> However RPO VN eliminates iftmp.2_13:
> Removing dead stmt iftmp.2_13 = .COND_DIV (_29, _4, a_lsm.10_11, 0);
>
> and we end up replacing iftmp.2_7 with a dead ssa_name in ifcvt_local_dce:
> FOR_EACH_VEC_ELT (redundant_ssa_names, i, name_pair)
> replace_uses_by (name_pair->first, name_pair->second);
> redundant_ssa_names.release ();
>
> resulting in incorrect IR, and segfault down the line.
>
> To avoid clashing of RPO VN with redunant_ssa_names, the patch simply moves
> ifcvt_local_dce before do_rpo_vn, which avoids the segfault.
> Does that look OK ?
> (Altho I guess, doing DCE after VN is better in principle)
Yes, I'd say just moving
FOR_EACH_VEC_ELT (redundant_ssa_names, i, name_pair)
replace_uses_by (name_pair->first, name_pair->second);
redundant_ssa_names.release ();
before rpo_vn makes more sense, no?
OK with that change.
Thanks,
Richard.
> Thanks,
> Prathamesh
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: PR97849: aarch64: ICE (segfault) during GIMPLE pass: ifcvt
2020-11-18 13:43 ` Richard Biener
@ 2020-11-24 1:32 ` Prathamesh Kulkarni
0 siblings, 0 replies; 3+ messages in thread
From: Prathamesh Kulkarni @ 2020-11-24 1:32 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc Patches
[-- Attachment #1: Type: text/plain, Size: 1953 bytes --]
On Wed, 18 Nov 2020 at 19:13, Richard Biener <rguenther@suse.de> wrote:
>
> On Wed, 18 Nov 2020, Prathamesh Kulkarni wrote:
>
> > Hi,
> > For the following test-case (slightly reduced from PR)
> > int a, b, c;
> >
> > int g() {
> > char i = 0;
> > for (c = 0; c <= 8; c++)
> > --i;
> >
> > while (b) {
> > _Bool f = i <= 0;
> > a = (a == 0) ? 0 : f / a;
> > }
> > }
> >
> > The compiler segfaults with -O1 -march=armv8.2-a+sve in ifcvt_local_dce.
> >
> > IIUC, the issue here is that tree-if-conv.c:predicate_rhs_code
> > processes the following statement:
> > iftmp.2_7 = a_lsm.10_11 != 0 ? iftmp.2_13 : 0;
> > and records <iftmp.2_7, iftmp.2_13> mapping.
> >
> > However RPO VN eliminates iftmp.2_13:
> > Removing dead stmt iftmp.2_13 = .COND_DIV (_29, _4, a_lsm.10_11, 0);
> >
> > and we end up replacing iftmp.2_7 with a dead ssa_name in ifcvt_local_dce:
> > FOR_EACH_VEC_ELT (redundant_ssa_names, i, name_pair)
> > replace_uses_by (name_pair->first, name_pair->second);
> > redundant_ssa_names.release ();
> >
> > resulting in incorrect IR, and segfault down the line.
> >
> > To avoid clashing of RPO VN with redunant_ssa_names, the patch simply moves
> > ifcvt_local_dce before do_rpo_vn, which avoids the segfault.
> > Does that look OK ?
> > (Altho I guess, doing DCE after VN is better in principle)
>
> Yes, I'd say just moving
>
> FOR_EACH_VEC_ELT (redundant_ssa_names, i, name_pair)
> replace_uses_by (name_pair->first, name_pair->second);
> redundant_ssa_names.release ();
>
> before rpo_vn makes more sense, no?
Ah indeed, thanks for the suggestion.
Committed the attached patch after bootstrap+test on x86_64, and
cross testing on aarch64*-*-* and arm*-*-*.
Thanks,
Prathamesh
>
> OK with that change.
> Thanks,
> Richard.
>
> > Thanks,
> > Prathamesh
> >
>
> --
> Richard Biener <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
> Germany; GF: Felix Imend
[-- Attachment #2: pr97849-3.diff --]
[-- Type: application/octet-stream, Size: 1679 bytes --]
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr97849.c b/gcc/testsuite/gcc.dg/tree-ssa/pr97849.c
new file mode 100644
index 00000000000..57a31e316a2
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr97849.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O1 -ftree-vectorize" } */
+/* { dg-additional-options "-march=armv8.2-a+sve" { target aarch64*-*-* } } */
+
+int a, b, c;
+
+int g() {
+ char i = 0;
+ for (c = 0; c <= 8; c++)
+ --i;
+
+ while (b) {
+ _Bool f = i <= 0;
+ a = (a == 0) ? 0 : f / a;
+ }
+}
diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c
index 2062758f40f..93effaa811b 100644
--- a/gcc/tree-if-conv.c
+++ b/gcc/tree-if-conv.c
@@ -2916,12 +2916,6 @@ ifcvt_local_dce (class loop *loop)
enum gimple_code code;
use_operand_p use_p;
imm_use_iterator imm_iter;
- std::pair <tree, tree> *name_pair;
- unsigned int i;
-
- FOR_EACH_VEC_ELT (redundant_ssa_names, i, name_pair)
- replace_uses_by (name_pair->first, name_pair->second);
- redundant_ssa_names.release ();
/* The loop has a single BB only. */
basic_block bb = loop->header;
@@ -3124,6 +3118,13 @@ tree_if_conversion (class loop *loop, vec<gimple *> *preds)
exit_bbs = BITMAP_ALLOC (NULL);
bitmap_set_bit (exit_bbs, single_exit (loop)->dest->index);
bitmap_set_bit (exit_bbs, loop->latch->index);
+
+ std::pair <tree, tree> *name_pair;
+ unsigned ssa_names_idx;
+ FOR_EACH_VEC_ELT (redundant_ssa_names, ssa_names_idx, name_pair)
+ replace_uses_by (name_pair->first, name_pair->second);
+ redundant_ssa_names.release ();
+
todo |= do_rpo_vn (cfun, loop_preheader_edge (loop), exit_bbs);
/* Delete dead predicate computations. */
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-11-24 1:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-18 11:34 PR97849: aarch64: ICE (segfault) during GIMPLE pass: ifcvt Prathamesh Kulkarni
2020-11-18 13:43 ` Richard Biener
2020-11-24 1:32 ` Prathamesh Kulkarni
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).