* [PATCH] Handle PHI nodes w/o a argument (PR ipa/80205). @ 2017-03-27 12:51 Martin Liška 2017-03-27 14:25 ` Richard Biener 0 siblings, 1 reply; 7+ messages in thread From: Martin Liška @ 2017-03-27 12:51 UTC (permalink / raw) To: GCC Patches; +Cc: Jan Hubicka [-- Attachment #1: Type: text/plain, Size: 264 bytes --] Hello. As described in the PR, we can create a PHI node in einline that has no argument. That can cause ICE in devirtualization and should be thus handled. Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. Ready to be installed? Martin [-- Attachment #2: 0001-Handle-PHI-nodes-w-o-a-argument-PR-ipa-80205.patch --] [-- Type: text/x-patch, Size: 1848 bytes --] From d0dc319a8df5d9f00434f54fef13b3dc427061e1 Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Mon, 27 Mar 2017 13:32:52 +0200 Subject: [PATCH] Handle PHI nodes w/o a argument (PR ipa/80205). gcc/testsuite/ChangeLog: 2017-03-27 Martin Liska <mliska@suse.cz> * g++.dg/ipa/pr80205.C: New test. gcc/ChangeLog: 2017-03-27 Martin Liska <mliska@suse.cz> PR ipa/80205 * ipa-polymorphic-call.c (walk_ssa_copies): Handle phi nodes w/o a argument. --- gcc/ipa-polymorphic-call.c | 2 +- gcc/testsuite/g++.dg/ipa/pr80205.C | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/ipa/pr80205.C diff --git a/gcc/ipa-polymorphic-call.c b/gcc/ipa-polymorphic-call.c index e690d05158d..89c008ee5c0 100644 --- a/gcc/ipa-polymorphic-call.c +++ b/gcc/ipa-polymorphic-call.c @@ -828,7 +828,7 @@ walk_ssa_copies (tree op, hash_set<tree> **global_visited = NULL) { gimple *phi = SSA_NAME_DEF_STMT (op); - if (gimple_phi_num_args (phi) > 2) + if (gimple_phi_num_args (phi) == 0 || gimple_phi_num_args (phi) > 2) goto done; if (gimple_phi_num_args (phi) == 1) op = gimple_phi_arg_def (phi, 0); diff --git a/gcc/testsuite/g++.dg/ipa/pr80205.C b/gcc/testsuite/g++.dg/ipa/pr80205.C new file mode 100644 index 00000000000..460bdcb02ca --- /dev/null +++ b/gcc/testsuite/g++.dg/ipa/pr80205.C @@ -0,0 +1,34 @@ +// PR ipa/80205 +// { dg-options "-fnon-call-exceptions --param early-inlining-insns=100 -O2" } + +class a +{ +public: + virtual ~a (); +}; +class b +{ +public: + template <typename c> b (c); + ~b () { delete d; } + void + operator= (b e) + { + b (e).f (*this); + } + void + f (b &e) + { + a g; + d = e.d; + e.d = &g; + } + a *d; +}; +void +h () +{ + b i = int(); + void j (); + i = j; +} -- 2.12.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Handle PHI nodes w/o a argument (PR ipa/80205). 2017-03-27 12:51 [PATCH] Handle PHI nodes w/o a argument (PR ipa/80205) Martin Liška @ 2017-03-27 14:25 ` Richard Biener 2017-03-27 14:27 ` Jeff Law 2017-03-27 14:28 ` Richard Biener 0 siblings, 2 replies; 7+ messages in thread From: Richard Biener @ 2017-03-27 14:25 UTC (permalink / raw) To: Martin Liška; +Cc: GCC Patches, Jan Hubicka On Mon, Mar 27, 2017 at 2:47 PM, Martin Liška <mliska@suse.cz> wrote: > Hello. > > As described in the PR, we can create a PHI node in einline that has no argument. > That can cause ICE in devirtualization and should be thus handled. > > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > > Ready to be installed? We shouldn't ever have a PHI w/o argument. > Martin ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Handle PHI nodes w/o a argument (PR ipa/80205). 2017-03-27 14:25 ` Richard Biener @ 2017-03-27 14:27 ` Jeff Law 2017-03-27 14:28 ` Richard Biener 1 sibling, 0 replies; 7+ messages in thread From: Jeff Law @ 2017-03-27 14:27 UTC (permalink / raw) To: Richard Biener, Martin Liška; +Cc: GCC Patches, Jan Hubicka On 03/27/2017 08:14 AM, Richard Biener wrote: > On Mon, Mar 27, 2017 at 2:47 PM, Martin LiÅ¡ka <mliska@suse.cz> wrote: >> Hello. >> >> As described in the PR, we can create a PHI node in einline that has no argument. >> That can cause ICE in devirtualization and should be thus handled. >> >> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >> >> Ready to be installed? > > We shouldn't ever have a PHI w/o argument. Yea. Sounds like there's a deeper problem somewhere. jeff ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Handle PHI nodes w/o a argument (PR ipa/80205). 2017-03-27 14:25 ` Richard Biener 2017-03-27 14:27 ` Jeff Law @ 2017-03-27 14:28 ` Richard Biener 2017-03-27 15:30 ` Jeff Law 2017-03-28 7:52 ` Martin Liška 1 sibling, 2 replies; 7+ messages in thread From: Richard Biener @ 2017-03-27 14:28 UTC (permalink / raw) To: Martin Liška; +Cc: GCC Patches, Jan Hubicka On Mon, Mar 27, 2017 at 4:14 PM, Richard Biener <richard.guenther@gmail.com> wrote: > On Mon, Mar 27, 2017 at 2:47 PM, Martin Liška <mliska@suse.cz> wrote: >> Hello. >> >> As described in the PR, we can create a PHI node in einline that has no argument. >> That can cause ICE in devirtualization and should be thus handled. >> >> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >> >> Ready to be installed? > > We shouldn't ever have a PHI w/o argument. ;; basic block 14, loop depth 0 ;; pred: # SR.2_19 = PHI <> <L4> [0.00%]: a::~a (&g); resx 12 ;; succ: 17 the CFG has not been cleaned up here (this block is unreachable). Hmm, I see we are called from fold_stmt. I suppose we process statements_to_fold before removing unreachable blocks to not walk over dead stmts... chicken-and-egg... Still not creating that PHI should be possible. Index: gcc/tree-inline.c =================================================================== --- gcc/tree-inline.c (revision 246500) +++ gcc/tree-inline.c (working copy) @@ -2344,6 +2344,13 @@ copy_phis_for_bb (basic_block bb, copy_b if (!virtual_operand_p (res)) { walk_tree (&new_res, copy_tree_body_r, id, NULL); + if (EDGE_COUNT (new_bb->preds) == 0) + { + /* Technically we'd want a SSA_DEFAULT_DEF here... */ + SSA_NAME_DEF_STMT (new_res) = gimple_build_nop (); + } + else + { new_phi = create_phi_node (new_res, new_bb); FOR_EACH_EDGE (new_edge, ei, new_bb->preds) { @@ -2389,6 +2396,7 @@ copy_phis_for_bb (basic_block bb, copy_b add_phi_arg (new_phi, new_arg, new_edge, locus); } + } } } >> Martin ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Handle PHI nodes w/o a argument (PR ipa/80205). 2017-03-27 14:28 ` Richard Biener @ 2017-03-27 15:30 ` Jeff Law 2017-03-28 7:52 ` Martin Liška 1 sibling, 0 replies; 7+ messages in thread From: Jeff Law @ 2017-03-27 15:30 UTC (permalink / raw) To: Richard Biener, Martin Liška; +Cc: GCC Patches, Jan Hubicka On 03/27/2017 08:27 AM, Richard Biener wrote: > On Mon, Mar 27, 2017 at 4:14 PM, Richard Biener > <richard.guenther@gmail.com> wrote: >> On Mon, Mar 27, 2017 at 2:47 PM, Martin LiÅ¡ka <mliska@suse.cz> wrote: >>> Hello. >>> >>> As described in the PR, we can create a PHI node in einline that has no argument. >>> That can cause ICE in devirtualization and should be thus handled. >>> >>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >>> >>> Ready to be installed? >> >> We shouldn't ever have a PHI w/o argument. > > ;; basic block 14, loop depth 0 > ;; pred: > # SR.2_19 = PHI <> > <L4> [0.00%]: > a::~a (&g); > resx 12 > ;; succ: 17 > > the CFG has not been cleaned up here (this block is unreachable). > > Hmm, I see we are called from fold_stmt. I suppose we process > statements_to_fold before removing unreachable blocks to not > walk over dead stmts... chicken-and-egg... > > Still not creating that PHI should be possible. Can't speak for this specific example, but could it be the case that the PHI exists with arguments, then becomes unreachable resulting in a PHI with no arguments? jeff ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Handle PHI nodes w/o a argument (PR ipa/80205). 2017-03-27 14:28 ` Richard Biener 2017-03-27 15:30 ` Jeff Law @ 2017-03-28 7:52 ` Martin Liška 2017-03-28 8:23 ` Richard Biener 1 sibling, 1 reply; 7+ messages in thread From: Martin Liška @ 2017-03-28 7:52 UTC (permalink / raw) To: Richard Biener; +Cc: GCC Patches, Jan Hubicka On 03/27/2017 04:27 PM, Richard Biener wrote: > On Mon, Mar 27, 2017 at 4:14 PM, Richard Biener > <richard.guenther@gmail.com> wrote: >> On Mon, Mar 27, 2017 at 2:47 PM, Martin LiÅ¡ka <mliska@suse.cz> wrote: >>> Hello. >>> >>> As described in the PR, we can create a PHI node in einline that has no argument. >>> That can cause ICE in devirtualization and should be thus handled. >>> >>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >>> >>> Ready to be installed? >> >> We shouldn't ever have a PHI w/o argument. > > ;; basic block 14, loop depth 0 > ;; pred: > # SR.2_19 = PHI <> > <L4> [0.00%]: > a::~a (&g); > resx 12 > ;; succ: 17 > > the CFG has not been cleaned up here (this block is unreachable). > > Hmm, I see we are called from fold_stmt. I suppose we process > statements_to_fold before removing unreachable blocks to not > walk over dead stmts... chicken-and-egg... > > Still not creating that PHI should be possible. I see, thanks for help. Patch fixes the issue, may I install it after regression tested? Thanks, Martin > > Index: gcc/tree-inline.c > =================================================================== > --- gcc/tree-inline.c (revision 246500) > +++ gcc/tree-inline.c (working copy) > @@ -2344,6 +2344,13 @@ copy_phis_for_bb (basic_block bb, copy_b > if (!virtual_operand_p (res)) > { > walk_tree (&new_res, copy_tree_body_r, id, NULL); > + if (EDGE_COUNT (new_bb->preds) == 0) > + { > + /* Technically we'd want a SSA_DEFAULT_DEF here... */ > + SSA_NAME_DEF_STMT (new_res) = gimple_build_nop (); > + } > + else > + { > new_phi = create_phi_node (new_res, new_bb); > FOR_EACH_EDGE (new_edge, ei, new_bb->preds) > { > @@ -2389,6 +2396,7 @@ copy_phis_for_bb (basic_block bb, copy_b > > add_phi_arg (new_phi, new_arg, new_edge, locus); > } > + } > } > } > > > >>> Martin ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Handle PHI nodes w/o a argument (PR ipa/80205). 2017-03-28 7:52 ` Martin Liška @ 2017-03-28 8:23 ` Richard Biener 0 siblings, 0 replies; 7+ messages in thread From: Richard Biener @ 2017-03-28 8:23 UTC (permalink / raw) To: Martin Liška; +Cc: GCC Patches, Jan Hubicka On Tue, Mar 28, 2017 at 9:45 AM, Martin Liška <mliska@suse.cz> wrote: > On 03/27/2017 04:27 PM, Richard Biener wrote: >> On Mon, Mar 27, 2017 at 4:14 PM, Richard Biener >> <richard.guenther@gmail.com> wrote: >>> On Mon, Mar 27, 2017 at 2:47 PM, Martin Liška <mliska@suse.cz> wrote: >>>> Hello. >>>> >>>> As described in the PR, we can create a PHI node in einline that has no argument. >>>> That can cause ICE in devirtualization and should be thus handled. >>>> >>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >>>> >>>> Ready to be installed? >>> >>> We shouldn't ever have a PHI w/o argument. >> >> ;; basic block 14, loop depth 0 >> ;; pred: >> # SR.2_19 = PHI <> >> <L4> [0.00%]: >> a::~a (&g); >> resx 12 >> ;; succ: 17 >> >> the CFG has not been cleaned up here (this block is unreachable). >> >> Hmm, I see we are called from fold_stmt. I suppose we process >> statements_to_fold before removing unreachable blocks to not >> walk over dead stmts... chicken-and-egg... >> >> Still not creating that PHI should be possible. > > I see, thanks for help. Patch fixes the issue, may I install it after > regression tested? I think it's progression, still not 100% safe as we have a SSA name with a GIMPLE_NOP definition that is not in any BB and the SSA name is not a default def. But yes, I've written a comment to that effect ;) Thus ok. An improvement would be to not copy unreachable regions at all... Thanks, Richard. > Thanks, > Martin > >> >> Index: gcc/tree-inline.c >> =================================================================== >> --- gcc/tree-inline.c (revision 246500) >> +++ gcc/tree-inline.c (working copy) >> @@ -2344,6 +2344,13 @@ copy_phis_for_bb (basic_block bb, copy_b >> if (!virtual_operand_p (res)) >> { >> walk_tree (&new_res, copy_tree_body_r, id, NULL); >> + if (EDGE_COUNT (new_bb->preds) == 0) >> + { >> + /* Technically we'd want a SSA_DEFAULT_DEF here... */ >> + SSA_NAME_DEF_STMT (new_res) = gimple_build_nop (); >> + } >> + else >> + { >> new_phi = create_phi_node (new_res, new_bb); >> FOR_EACH_EDGE (new_edge, ei, new_bb->preds) >> { >> @@ -2389,6 +2396,7 @@ copy_phis_for_bb (basic_block bb, copy_b >> >> add_phi_arg (new_phi, new_arg, new_edge, locus); >> } >> + } >> } >> } >> >> >> >>>> Martin > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-03-28 8:12 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-03-27 12:51 [PATCH] Handle PHI nodes w/o a argument (PR ipa/80205) Martin Liška 2017-03-27 14:25 ` Richard Biener 2017-03-27 14:27 ` Jeff Law 2017-03-27 14:28 ` Richard Biener 2017-03-27 15:30 ` Jeff Law 2017-03-28 7:52 ` Martin Liška 2017-03-28 8:23 ` 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).