| Message ID | 20180320133205.GE3522@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | C++ PATCH for c++/84978, ICE with NRVO | expand |
On Tue, Mar 20, 2018 at 9:32 AM, Marek Polacek <polacek@redhat.com> wrote: > We started crashing on this test with r258592 which added cp_get_callee_fndecl > in <case AGGR_INIT_EXPR> in cp_genericize_r. > > This ICE apparently depends on whether we perform NRVO or not. If the size of > S is <=16B we pass it in registers and it compiles fine. But if the size of S > is >16B, then we pass in memory, and we NRV-optimize. That means that > s.fn (); > is turned by finalize_nrv into > <retval>.fn (); > > Then the newly added call to cp_get_callee_fndecl calls maybe_constant_init, Oops, I forgot that cp_get_callee_fndecl would call maybe_constant_init, I was just using it to handle both CALL_EXPR and AGGR_INIT_EXPR. And in fact it looks like we don't really want that for the other users, either. I think I'll remove it. > 2018-03-20 Marek Polacek <polacek@redhat.com> > > PR c++/84978 > * constexpr.c (cxx_eval_constant_expression): Handle the case when > a RESULT_DECL isn't in the hash map. But your patch is also good. OK. Jason
On Tue, Mar 20, 2018 at 11:55 AM, Jason Merrill <jason@redhat.com> wrote: > On Tue, Mar 20, 2018 at 9:32 AM, Marek Polacek <polacek@redhat.com> wrote: >> We started crashing on this test with r258592 which added cp_get_callee_fndecl >> in <case AGGR_INIT_EXPR> in cp_genericize_r. >> >> This ICE apparently depends on whether we perform NRVO or not. If the size of >> S is <=16B we pass it in registers and it compiles fine. But if the size of S >> is >16B, then we pass in memory, and we NRV-optimize. That means that >> s.fn (); >> is turned by finalize_nrv into >> <retval>.fn (); >> >> Then the newly added call to cp_get_callee_fndecl calls maybe_constant_init, > > Oops, I forgot that cp_get_callee_fndecl would call > maybe_constant_init, I was just using it to handle both CALL_EXPR and > AGGR_INIT_EXPR. And in fact it looks like we don't really want that > for the other users, either. I think I'll remove it. Thus. commit 06d961cdae77d73332845eaa2ed8d1ea1867dc73 Author: Jason Merrill <jason@redhat.com> Date: Tue Mar 20 08:51:18 2018 -0400 PR c++/84978, ICE with NRVO. * cvt.c (cp_get_fndecl_from_callee): Add fold parameter. (cp_get_callee_fndecl_nofold): New. * cp-gimplify.c (cp_genericize_r): Use it instead. * call.c (check_self_delegation): Likewise. diff --git a/gcc/cp/call.c b/gcc/cp/call.c index 23d4f82a1a0..dbdb8d5812d 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -8739,7 +8739,7 @@ check_self_delegation (tree ret) { if (TREE_CODE (ret) == TARGET_EXPR) ret = TARGET_EXPR_INITIAL (ret); - tree fn = cp_get_callee_fndecl (ret); + tree fn = cp_get_callee_fndecl_nofold (ret); if (fn && DECL_ABSTRACT_ORIGIN (fn) == current_function_decl) error ("constructor delegates to itself"); } diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c index 332ff2bbb05..3edecf24c92 100644 --- a/gcc/cp/cp-gimplify.c +++ b/gcc/cp/cp-gimplify.c @@ -1521,7 +1521,7 @@ cp_genericize_r (tree *stmt_p, int *walk_subtrees, void *data) version is inlinable, a direct call to this version can be made otherwise the call should go through the dispatcher. */ { - tree fn = cp_get_callee_fndecl (stmt); + tree fn = cp_get_callee_fndecl_nofold (stmt); if (fn && DECL_FUNCTION_VERSIONED (fn) && (current_function_decl == NULL || !targetm.target_option.can_inline_p (current_function_decl, diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 727822e36da..9befde3329d 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -6110,7 +6110,8 @@ extern tree cp_convert_and_check (tree, tree, tsubst_flags_t); extern tree cp_fold_convert (tree, tree); extern tree cp_get_callee (tree); extern tree cp_get_callee_fndecl (tree); -extern tree cp_get_fndecl_from_callee (tree); +extern tree cp_get_callee_fndecl_nofold (tree); +extern tree cp_get_fndecl_from_callee (tree, bool fold = true); extern tree convert_to_void (tree, impl_conv_void, tsubst_flags_t); extern tree convert_force (tree, tree, int, diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c index 40e7576f23c..9b53fa3067d 100644 --- a/gcc/cp/cvt.c +++ b/gcc/cp/cvt.c @@ -941,7 +941,7 @@ cp_get_callee (tree call) if we can. */ tree -cp_get_fndecl_from_callee (tree fn) +cp_get_fndecl_from_callee (tree fn, bool fold /* = true */) { if (fn == NULL_TREE) return fn; @@ -951,7 +951,8 @@ cp_get_fndecl_from_callee (tree fn) if (type == unknown_type_node) return NULL_TREE; gcc_assert (POINTER_TYPE_P (type)); - fn = maybe_constant_init (fn); + if (fold) + fn = maybe_constant_init (fn); STRIP_NOPS (fn); if (TREE_CODE (fn) == ADDR_EXPR) { @@ -971,6 +972,14 @@ cp_get_callee_fndecl (tree call) return cp_get_fndecl_from_callee (cp_get_callee (call)); } +/* As above, but not using the constexpr machinery. */ + +tree +cp_get_callee_fndecl_nofold (tree call) +{ + return cp_get_fndecl_from_callee (cp_get_callee (call), false); +} + /* Subroutine of convert_to_void. Warn if we're discarding something with attribute [[nodiscard]]. */
diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c index 894bcd0bb3e..1f8ece89730 100644 --- gcc/cp/constexpr.c +++ gcc/cp/constexpr.c @@ -4111,7 +4111,15 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t, /* We ask for an rvalue for the RESULT_DECL when indirecting through an invisible reference, or in named return value optimization. */ - return (*ctx->values->get (t)); + if (tree *p = ctx->values->get (t)) + return *p; + else + { + if (!ctx->quiet) + error ("%qE is not a constant expression", t); + *non_constant_p = true; + } + break; case VAR_DECL: if (DECL_HAS_VALUE_EXPR_P (t)) diff --git gcc/testsuite/g++.dg/opt/nrv19.C gcc/testsuite/g++.dg/opt/nrv19.C index e69de29bb2d..385593cc90c 100644 --- gcc/testsuite/g++.dg/opt/nrv19.C +++ gcc/testsuite/g++.dg/opt/nrv19.C @@ -0,0 +1,15 @@ +// PR c++/84978 +// { dg-do compile } + +struct S { + void (*fn)(); + int a[10]; +}; + +S +foo () +{ + S s; + s.fn (); + return s; +}