* 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).