Message ID | 1377632573-14453-2-git-send-email-adam@jessamine.co.uk |
---|---|
State | New |
Headers | show |
On 08/27/2013 03:42 PM, Adam Butcher wrote:
> + vec_safe_push (argvec, arg);
I bet we want convert_from_reference in the non-generic lambda case, too.
OK with that change.
Jason
On 01.09.2013 21:22, Jason Merrill wrote: > On 08/27/2013 03:42 PM, Adam Butcher wrote: >> + vec_safe_push (argvec, arg); > > I bet we want convert_from_reference in the non-generic lambda case, > too. > > OK with that change. > I think I had made that change originally to keep the two impls the same and I hit issues with non-generic lambdas. But I can't remember the details. I'll try again. If it works with convert_from_reference in both cases should I push or should I sort out the parameter pack conversion op issue and roll that up into this?
On 09/02/2013 02:30 PM, Adam Butcher wrote: > I think I had made that change originally to keep the two impls the same > and I hit issues with non-generic lambdas. But I can't remember the > details. I'll try again. If it works with convert_from_reference in > both cases should I push or should I sort out the parameter pack > conversion op issue and roll that up into this? I think roll them together, since that patch rewrites parts of this one. Jason
On 02.09.2013 19:34, Jason Merrill wrote: > On 09/02/2013 02:30 PM, Adam Butcher wrote: >> On 01.09.2013 21:22, Jason Merrill wrote: >>> I bet we want convert_from_reference in the non-generic lambda >>> case, too. >>> >> I think I had made that change originally to keep the two impls the >> same >> and I hit issues with non-generic lambdas. But I can't remember the >> details. I'll try again. >> Okay, finally got around to trying this again. With convert_from_reference in the non-generic case, the code compiles okay but SEGVs on the attempt to branch to '_FUN'. │105 auto lf0 = [] (float& a, int const& b) { return a += b; }; │106 │107 INVOKEi (lf, float, 7, 0); >│108 AS_FUNi (lf, float, 7, 0); │109 AS_PTRi (lf, float, int, 7, 0); │0x404500 <main()+14687> mov %eax,-0x4bc(%rbp) │0x404506 <main()+14693> mov 0x36f0(%rip),%eax # 0x407bfc │0x40450c <main()+14699> mov %eax,-0x4c0(%rbp) │0x404512 <main()+14705> movl $0x7,-0x2a4(%rbp) │0x40451c <main()+14715> lea -0x2a4(%rbp),%rdx │0x404523 <main()+14722> lea -0x4bc(%rbp),%rax │0x40452a <main()+14729> mov %rdx,%rsi │0x40452d <main()+14732> mov %rax,%rdi >│0x404530 <main()+14735> callq 0x400934 <<lambda(float&, int const&)>::_FUN(float &, const int &)> >> If it works with convert_from_reference in >> both cases should I push or should I sort out the parameter pack >> conversion op issue and roll that up into this? > > I think roll them together, since that patch rewrites parts of this > one. > Will assume, for now, that the convert_from_reference call is not wanted in the non-generic case (maybe something to do with using 'build_call_a' instead of 'build_nt_call_vec' or the convert_from_reference on the call itself?) and will focus on the parameter pack stuff (when I get a chance). Cheers, Adam
On 09/02/2013 05:18 PM, Adam Butcher wrote: > Will assume, for now, that the convert_from_reference call is not wanted > in the non-generic case (maybe something to do with using 'build_call_a' > instead of 'build_nt_call_vec' or the convert_from_reference on the call > itself?) Ah, yes; we are passing references through directly as pointers rather than doing the equivalent of &*. > and will focus on the parameter pack stuff (when I get a chance). Sounds good. Jason
On 03.09.2013 04:50, Jason Merrill wrote: > On 09/02/2013 05:18 PM, Adam Butcher wrote: > > will focus on the parameter pack stuff (when I get a chance). > > > Sounds good. > I had a quick hack at getting pack expansion working for the conversion op. The syntactic side seems to be okay. It gets all the way to finalizing the tu. It generates suitable diagnostics if I force warnings in various places in my testcase. I've done what amounts to the following (diff hand edited to removing noisy debug logging and temporaries): ------------------ @@ -795,20 +794,39 @@ maybe_add_lambda_conv_op (tree type) while (src) { - if (FUNCTION_PARAMETER_PACK_P (src)) - return; + tree new_node = copy_node (src); if (!fn_args) - fn_args = tgt = copy_node (src); + fn_args = tgt = new_node; else { - TREE_CHAIN (tgt) = copy_node (src); - tgt = TREE_CHAIN (tgt); + TREE_CHAIN (tgt) = new_node; + tgt = new_node; } mark_exp_read (tgt); + + if (FUNCTION_PARAMETER_PACK_P (tgt)) + vec_safe_push (argvec, make_pack_expansion (tgt)); + else vec_safe_push (argvec, generic_lambda_p ? convert_from_reference (tgt) : tgt); src = TREE_CHAIN (src); } ------------------ Problem is that no RTL is set for the incoming parms in the instantiation of the expansion. It ICEs in gimple_expand_cfg because 'DECL_RTL_IF_SET (var)' returns nullptr for the incoming parms resulting in a failed assertion that SA.partition_to_pseudo[i] is non-null. What follows below is basically a dump of various info that may help you to point me in the right direction or may be completely useless or unnecessary to you. Any ideas appreciated. Cheers, Adam The error diagnostic is: /home/ajb/t7-variadic-ptr.cpp: In static member function ‘static decltype (((main()::<lambda(P ...)>)0u).operator()(main::__lambda1::_FUN::<unnamed> ...)) main()::<lambda(P ...)>::_FUN(P ...) [with P = {double, double, double}; decltype (((main()::<lambda(P ...)>)0u).operator()(main::__lambda1::_FUN::<unnamed> ...)) = float]’: /home/ajb/t7-variadic-ptr.cpp:13:37: internal compiler error: in gimple_expand_cfg, at cfgexpand.c:4649 auto g = [] <typename... P> (P...) { return 3.f; }; ^ This only occurs if I instantiate the conversion op. I added the following tracing to gimple_expand_cfg: -------------- @@ -4635,9 +4635,17 @@ gimple_expand_cfg (void) { tree var = SSA_NAME_VAR (partition_to_var (SA.map, i)); + debug_tree (var); + + if (TREE_CODE (var) != VAR_DECL) + fprintf (stderr, "SA.partition_to_pseudo[%d] == %p\n", i, SA.partition_to_pseudo[i]); + if (TREE_CODE (var) != VAR_DECL && !SA.partition_to_pseudo[i]) + { SA.partition_to_pseudo[i] = DECL_RTL_IF_SET (var); + fprintf (stderr, "SA.partition_to_pseudo[%d] => %p\n", i, SA.partition_to_pseudo[i]); + } gcc_assert (SA.partition_to_pseudo[i]); /* If this decl was marked as living in multiple places, reset -------------- I expected the instantiated parm_decl for the pack expansion parms to look similar to this (from a 'normal' non-pack parm) ... <parm_decl 0x7f38340e1f80 D.2134 type <real_type 0x7f3833f83f18 float type_6 SF size <integer_cst 0x7f3833f85340 constant 32> unit size <integer_cst 0x7f3833f85360 constant 4> align 32 symtab 0 alias set -1 canonical type 0x7f3833f83f18 precision 32 pointer_to_this <pointer_type 0x7f3833f8d150>> used SF file /home/ajb/t7-variadic-ptr.cpp line 12 col 30 size <integer_cst 0x7f3833f85340 32> unit size <integer_cst 0x7f3833f85360 4> align 32 context <function_decl 0x7f38340e0900 _FUN> (mem/c:SF (plus:DI (reg/f:DI 70 virtual-stack-vars) (const_int -20 [0xffffffffffffffec])) [0 D.2134+0 S4 A32]) arg-type <real_type 0x7f3833f83f18 float> incoming-rtl (reg:SF 21 xmm0 [ D.2134 ])> ... but instead it looks like this: <parm_decl 0x7f38340e4600 D.2145 type <real_type 0x7f3833f8d000 double type_6 DF size <integer_cst 0x7f3833f67fc0 constant 64> unit size <integer_cst 0x7f3833f67fe0 constant 8> align 64 symtab 0 alias set -1 canonical type 0x7f3833f8d000 precision 64 pointer_to_this <pointer_type 0x7f3833f8d1f8>> used VOID file /home/ajb/t7-variadic-ptr.cpp line 13 col 34 align 8 arg-type <real_type 0x7f3833f8d000 double> chain <parm_decl 0x7f38340e4680 D.2146>> Everything under the tree type seems to be 'default' and I note that context has gone (it is there in the pack expansion expression prior to instantiation). I'm not sure if this is relevant. (note I was passing 3 doubles into the variadic template and a float into the 'plain' single arg template) Here's some dumps of the expansion creation from maybe_add_lambda_conv_op: SRC => <parm_decl 0x7f0ae009e880 D.2062 type <type_pack_expansion 0x7f0ae00a0498 type <template_type_parm 0x7f0ae00a03f0 P VOID align 8 symtab 0 alias set -1 canonical type 0x7f0ae008c150 index 0 level 1 orig_level 1 chain <type_decl 0x7f0ae0098cf0 P>> type_0 type_6 VOID align 8 symtab 0 alias set -1 structural equality> decl_1 VOID file /home/ajb/t7-variadic-ptr.cpp line 13 col 34 align 8 context <function_decl 0x7f0ae009f800 operator()> > NEW_NODE => <parm_decl 0x7f0ae009ea00 D.2068 type <type_pack_expansion 0x7f0ae00a0498 type <template_type_parm 0x7f0ae00a03f0 P VOID align 8 symtab 0 alias set -1 canonical type 0x7f0ae008c150 index 0 level 1 orig_level 1 chain <type_decl 0x7f0ae0098cf0 P>> type_0 type_6 VOID align 8 symtab 0 alias set -1 structural equality> decl_1 VOID file /home/ajb/t7-variadic-ptr.cpp line 13 col 34 align 8 context <function_decl 0x7f0ae009f800 operator()> > MAKE_PACK_EXPANSION => <expr_pack_expansion 0x7f0ae009d270 type <type_pack_expansion 0x7f0ae00a0498 type <template_type_parm 0x7f0ae00a03f0 P VOID align 8 symtab 0 alias set -1 canonical type 0x7f0ae008c150 index 0 level 1 orig_level 1 chain <type_decl 0x7f0ae0098cf0 P>> type_0 type_6 VOID align 8 symtab 0 alias set -1 structural equality> arg 0 <parm_decl 0x7f0ae009ea00 D.2068 type <type_pack_expansion 0x7f0ae00a0498> decl_1 VOID file /home/ajb/t7-variadic-ptr.cpp line 13 col 34 align 8 context <function_decl 0x7f0ae009f800 operator()> > arg 1 <tree_list 0x7f0ae00a12d0 value <parm_decl 0x7f0ae009ea00 D.2068>>> (note the decl context is updated to _FUN instead of operator() after this was logged).
On 09/03/2013 03:50 PM, Adam Butcher wrote: > Problem is that no RTL is set for the incoming parms in the > instantiation of the expansion. It ICEs in gimple_expand_cfg because 'DECL_RTL_IF_SET > (var)' returns nullptr for the incoming parms resulting in a failed assertion that > SA.partition_to_pseudo[i] is non-null. Sounds like a problem with how _FUN's parameters are instantiated. I'm not sure why it would be special. Does using a function parameter pack in the lambda body work currently? If so, how are the expanded parameters different? Jason
On 03.09.2013 22:25, Jason Merrill wrote: > On 09/03/2013 03:50 PM, Adam Butcher wrote: >> Problem is that no RTL is set for the incoming parms in the >> instantiation of the expansion. It ICEs in gimple_expand_cfg >> because 'DECL_RTL_IF_SET >> (var)' returns nullptr for the incoming parms resulting in a failed >> assertion that >> SA.partition_to_pseudo[i] is non-null. > > Sounds like a problem with how _FUN's parameters are instantiated. > I'm not sure why it would be special. > > Does using a function parameter pack in the lambda body work > currently? If so, how are the expanded parameters different? > Yes it does work. And I think (hope) I've cracked it; make_pack_expansion needs to be called in the body of the thunk (i.e. after start_preparsed_function). If I butcher the implementation so that I rewrite any parameter packs with their expansion prior to building the call in return statement, it seems to work. Certainly the following does what I expect it to, the trees look right and, more importantly, it doesn't ICE! auto g = [] <typename... P> (P...) -> float { return 3.f; }; float (*pg3) (double, double, double) = g; return pg3(1,2,3); I'll try to clean up what I've got then submit a rolled up patch. Cheers, Adam
On 05.09.2013 20:14, Adam Butcher wrote: > On 03.09.2013 22:25, Jason Merrill wrote: >> On 09/03/2013 03:50 PM, Adam Butcher wrote: >>> Problem is that no RTL is set for the incoming parms in the >>> instantiation of the expansion. It ICEs in gimple_expand_cfg >>> because 'DECL_RTL_IF_SET >>> (var)' returns nullptr for the incoming parms resulting in a failed >>> assertion that >>> SA.partition_to_pseudo[i] is non-null. >> >> Sounds like a problem with how _FUN's parameters are instantiated. >> I'm not sure why it would be special. >> >> Does using a function parameter pack in the lambda body work >> currently? If so, how are the expanded parameters different? >> > Yes it does work. And I think (hope) I've cracked it; > make_pack_expansion needs to be called in the body of the thunk (i.e. > after start_preparsed_function). > Okay, this worked because it had the side effect of setting PACK_EXPANSION_LOCAL_P on the expansion. Doing that at the top, in the existing code seems to have the same effect. Cheers, Adam
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c index d5d2912..ac9dbd7 100644 --- a/gcc/cp/decl2.c +++ b/gcc/cp/decl2.c @@ -507,8 +507,9 @@ check_member_template (tree tmpl) || (TREE_CODE (decl) == TYPE_DECL && MAYBE_CLASS_TYPE_P (TREE_TYPE (decl)))) { - /* The parser rejects template declarations in local classes. */ - gcc_assert (!current_function_decl); + /* The parser rejects template declarations in local classes + (with the exception of generic lambdas). */ + gcc_assert (!current_function_decl || LAMBDA_FUNCTION_P (decl)); /* The parser rejects any use of virtual in a function template. */ gcc_assert (!(TREE_CODE (decl) == FUNCTION_DECL && DECL_VIRTUAL_P (decl))); diff --git a/gcc/cp/lambda.c b/gcc/cp/lambda.c index a53e692..e9bc7c5 100644 --- a/gcc/cp/lambda.c +++ b/gcc/cp/lambda.c @@ -196,7 +196,7 @@ lambda_function (tree lambda) /*protect=*/0, /*want_type=*/false, tf_warning_or_error); if (lambda) - lambda = BASELINK_FUNCTIONS (lambda); + lambda = STRIP_TEMPLATE (get_first_fn (lambda)); return lambda; } @@ -759,6 +759,10 @@ maybe_add_lambda_conv_op (tree type) if (processing_template_decl) return; + bool generic_lambda_p + = (DECL_TEMPLATE_INFO (callop) + && DECL_TEMPLATE_RESULT (DECL_TI_TEMPLATE (callop)) == callop); + if (DECL_INITIAL (callop) == NULL_TREE) { /* If the op() wasn't instantiated due to errors, give up. */ @@ -766,7 +770,54 @@ maybe_add_lambda_conv_op (tree type) return; } - stattype = build_function_type (TREE_TYPE (TREE_TYPE (callop)), + tree fn_result = TREE_TYPE (TREE_TYPE (callop)); + tree fn_args = copy_list (DECL_CHAIN (DECL_ARGUMENTS (callop))); + + if (generic_lambda_p) + { + /* Construct the dependent member call for the static member function + '_FUN' and remove 'auto' from its return type to allow for simple + implementation of the conversion operator. */ + + tree instance = build_nop (type, null_pointer_node); + argvec = make_tree_vector (); + for (arg = fn_args; arg; arg = DECL_CHAIN (arg)) + { + mark_exp_read (arg); + vec_safe_push (argvec, convert_from_reference (arg)); + } + + tree objfn = build_min (COMPONENT_REF, NULL_TREE, + instance, DECL_NAME (callop), NULL_TREE); + call = build_nt_call_vec (objfn, argvec); + + if (type_uses_auto (fn_result)) + { + ++processing_template_decl; + fn_result = finish_decltype_type + (call, /*id_expression_or_member_access_p=*/false, + tf_warning_or_error); + --processing_template_decl; + } + } + else + { + arg = build1 (NOP_EXPR, TREE_TYPE (DECL_ARGUMENTS (callop)), + null_pointer_node); + argvec = make_tree_vector (); + argvec->quick_push (arg); + for (arg = fn_args; arg; arg = DECL_CHAIN (arg)) + { + mark_exp_read (arg); + vec_safe_push (argvec, arg); + } + call = build_call_a (callop, argvec->length (), argvec->address ()); + CALL_FROM_THUNK_P (call) = 1; + if (MAYBE_CLASS_TYPE_P (TREE_TYPE (call))) + call = build_cplus_new (TREE_TYPE (call), call, tf_warning_or_error); + } + + stattype = build_function_type (fn_result, FUNCTION_ARG_CHAIN (callop)); /* First build up the conversion op. */ @@ -794,6 +845,9 @@ maybe_add_lambda_conv_op (tree type) if (nested) DECL_INTERFACE_KNOWN (fn) = 1; + if (generic_lambda_p) + fn = add_inherited_template_parms (fn, DECL_TI_TEMPLATE (callop)); + add_method (type, fn, NULL_TREE); /* Generic thunk code fails for varargs; we'll complain in mark_used if @@ -820,8 +874,8 @@ maybe_add_lambda_conv_op (tree type) DECL_NOT_REALLY_EXTERN (fn) = 1; DECL_DECLARED_INLINE_P (fn) = 1; DECL_STATIC_FUNCTION_P (fn) = 1; - DECL_ARGUMENTS (fn) = copy_list (DECL_CHAIN (DECL_ARGUMENTS (callop))); - for (arg = DECL_ARGUMENTS (fn); arg; arg = DECL_CHAIN (arg)) + DECL_ARGUMENTS (fn) = fn_args; + for (arg = fn_args; arg; arg = DECL_CHAIN (arg)) { /* Avoid duplicate -Wshadow warnings. */ DECL_NAME (arg) = NULL_TREE; @@ -830,6 +884,9 @@ maybe_add_lambda_conv_op (tree type) if (nested) DECL_INTERFACE_KNOWN (fn) = 1; + if (generic_lambda_p) + fn = add_inherited_template_parms (fn, DECL_TI_TEMPLATE (callop)); + add_method (type, fn, NULL_TREE); if (nested) @@ -852,27 +909,15 @@ maybe_add_lambda_conv_op (tree type) } body = begin_function_body (); compound_stmt = begin_compound_stmt (0); - - arg = build1 (NOP_EXPR, TREE_TYPE (DECL_ARGUMENTS (callop)), - null_pointer_node); - argvec = make_tree_vector (); - argvec->quick_push (arg); - for (arg = DECL_ARGUMENTS (statfn); arg; arg = DECL_CHAIN (arg)) - { - mark_exp_read (arg); - vec_safe_push (argvec, arg); - } - call = build_call_a (callop, argvec->length (), argvec->address ()); - CALL_FROM_THUNK_P (call) = 1; - if (MAYBE_CLASS_TYPE_P (TREE_TYPE (call))) - call = build_cplus_new (TREE_TYPE (call), call, tf_warning_or_error); call = convert_from_reference (call); finish_return_stmt (call); finish_compound_stmt (compound_stmt); finish_function_body (body); - expand_or_defer_fn (finish_function (2)); + fn = finish_function (/*inline*/2); + if (!generic_lambda_p) + expand_or_defer_fn (fn); /* Generate the body of the conversion op. */ @@ -888,7 +933,9 @@ maybe_add_lambda_conv_op (tree type) finish_compound_stmt (compound_stmt); finish_function_body (body); - expand_or_defer_fn (finish_function (2)); + fn = finish_function (/*inline*/2); + if (!generic_lambda_p) + expand_or_defer_fn (fn); if (nested) pop_function_context (); diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 923277b..1f0c2c2 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -8783,6 +8783,7 @@ cp_parser_lambda_introducer (cp_parser* parser, tree lambda_expr) /* Parse the (optional) middle of a lambda expression. lambda-declarator: + < template-parameter-list [opt] > ( parameter-declaration-clause [opt] ) attribute-specifier [opt] mutable [opt] @@ -8802,9 +8803,30 @@ cp_parser_lambda_declarator_opt (cp_parser* parser, tree lambda_expr) tree param_list = void_list_node; tree attributes = NULL_TREE; tree exception_spec = NULL_TREE; + tree template_param_list = NULL_TREE; - /* The lambda-declarator is optional, but must begin with an opening - parenthesis if present. */ + /* The template-parameter-list is optional, but must begin with + an opening angle if present. */ + if (cp_lexer_next_token_is (parser->lexer, CPP_LESS)) + { + if (cxx_dialect < cxx1y) + pedwarn (parser->lexer->next_token->location, 0, + "lambda templates are only available with " + "-std=c++1y or -std=gnu++1y"); + + cp_lexer_consume_token (parser->lexer); + + template_param_list = cp_parser_template_parameter_list (parser); + + cp_parser_skip_to_end_of_template_parameter_list (parser); + + /* We just processed one more parameter list. */ + ++parser->num_template_parameter_lists; + } + + /* The parameter-declaration-clause is optional (unless + template-parameter-list was given), but must begin with an + opening parenthesis if present. */ if (cp_lexer_next_token_is (parser->lexer, CPP_OPEN_PAREN)) { cp_lexer_consume_token (parser->lexer); @@ -8847,6 +8869,8 @@ cp_parser_lambda_declarator_opt (cp_parser* parser, tree lambda_expr) trailing-return-type in case of decltype. */ pop_bindings_and_leave_scope (); } + else if (template_param_list != NULL_TREE) // generate diagnostic + cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN); /* Create the function call operator. @@ -8890,6 +8914,12 @@ cp_parser_lambda_declarator_opt (cp_parser* parser, tree lambda_expr) DECL_ARTIFICIAL (fco) = 1; /* Give the object parameter a different name. */ DECL_NAME (DECL_ARGUMENTS (fco)) = get_identifier ("__closure"); + if (template_param_list) + { + fco = finish_member_template_decl (fco); + finish_template_decl (template_param_list); + --parser->num_template_parameter_lists; + } } finish_member_declaration (fco); @@ -9012,7 +9042,11 @@ cp_parser_lambda_body (cp_parser* parser, tree lambda_expr) finish_lambda_scope (); /* Finish the function and generate code for it if necessary. */ - expand_or_defer_fn (finish_function (/*inline*/2)); + tree fn = finish_function (/*inline*/2); + + /* Only expand if the call op is not a template. */ + if (!DECL_TEMPLATE_INFO (fco)) + expand_or_defer_fn (fn); } parser->local_variables_forbidden_p = local_variables_forbidden_p; diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index e937318..ba841ca 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -9102,7 +9102,9 @@ instantiate_class_template_1 (tree type) tree decl = lambda_function (type); if (decl) { - instantiate_decl (decl, false, false); + if (!DECL_TEMPLATE_INFO (decl) + || DECL_TEMPLATE_RESULT (DECL_TI_TEMPLATE (decl)) != decl) + instantiate_decl (decl, false, false); /* We need to instantiate the capture list from the template after we've instantiated the closure members, but before we