_Cilk_spawn and _Cilk_sync for C++

Hi Jason,
    Please see my responses below

> -----Original Message-----
> From: Jason Merrill [mailto:jason@redhat.com]
> Sent: Friday, November 22, 2013 10:51 AM
> To: Iyer, Balaji V; gcc-patches@gcc.gnu.org
> Cc: Jeff Law
> Subject: Re: _Cilk_spawn and _Cilk_sync for C++
> 
> On 11/21/2013 05:40 PM, Iyer, Balaji V wrote:
> > +/* Returns a TRY_CATCH_EXPR that will encapsulate BODY, EXCEPT_DATA
> and
> > +   EXCEPT_FLAG.  */
> > +
> > +tree
> > +create_cilk_try_catch (tree except_flag, tree except_data, tree body)
> > +{
> > +  tree catch_list = alloc_stmt_list ();
> > +  append_to_statement_list (except_flag, &catch_list);
> > +  append_to_statement_list (except_data, &catch_list);
> > +  append_to_statement_list (do_begin_catch (), &catch_list);
> > +  append_to_statement_list (build_throw (NULL_TREE), &catch_list);
> > +  tree catch_tf_expr = build_stmt (EXPR_LOCATION (body),
> TRY_FINALLY_EXPR,
> > +				   catch_list, do_end_catch (NULL_TREE));
> > +  catch_list = build2 (CATCH_EXPR, void_type_node, NULL_TREE,
> > +		       catch_tf_expr);
> > +  tree try_catch_expr = build_stmt (EXPR_LOCATION (body),
> TRY_CATCH_EXPR,
> > +				    body, catch_list);
> > +  return try_catch_expr;
> > +}
> 
> I had in mind something less cilk-specific: a function that takes two tree
> operands, one for the body and one for the throwing path.
> Basically, make catch_list a parameter and move the first two appends back
> into the calling function.
> 

This is fixed as you suggested.

> > The reason is that, when you have something like this:
> >
> > _Cilk_spawn [=]  { <body> } ();
> >
> > I need to capture the function call (which in this case is the whole function)
> and throw it into a nested function.  The nested function implementation is
> shared with C. If the function is stored in a variable then I can just send that
> out to the nested function. I have added another constraint to make sure the
> function is a spawning function, this way we can reduce more cases were
> they are stored to a variable. The reason why I added this check in
> finish_call_expr is that it seemed to be most straight-forward for me and
> only place where I could do with least disruption (code-changes).
> 
> It looks like you're transforming any
> 
> [...] {...} (...);
> 
> into
> 
> auto lambda = [...]{...};
> lambda(...);
> 
> which has significantly different semantics, particularly in terms of the
> lifetime of the lambda object.  In some of the Cilk online documentation, I
> see:
> 
> > When spawning named lambda functions, be careful that the lifespan of
> the lambda extends at least until the next sync, or else the destructor for the
> lambda will race with the spawned call. For example:
> > double i = g();
> > if (some condition) {
> >    // named lambda with value capture of i
> >    auto f = [=i]() { double d = sin(i); f(d); };
> >    cilk_spawn f();
> > } // Ouch! destructor for f is in parallel with spawned call.
> 
> This would seem to apply even more to unnamed lambda functions, since
> normally they would be destroyed at the end of the full-expression, which
> must always be before the sync.  Does the Intel compiler implicitly extend
> the lifetime of a lambda called by cilk spawn?  In any case, we really don't
> want to do this for all calls to unnamed lambdas just because we turned on
> cilk mode.
> 

I have fixed this issue.  My function to map the variable's context from the spawner to the spawn helper function was going into the lambda function. I made it stop by adding a language specific copy_tree_body (basically stop going into the lambda function's body for C++ and for the rest of the times just use copy_tree_body_r, no code duplicating  is done between the two) that and it works fine now.

The fixed patch is attached and here are the fixed ChangeLogs:

gcc/c-family/ChangeLog
2013-11-25  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        * cilk.c (cilk_outline): Replaced a call to copy_tree_body_r with the
        language specific one.

gcc/c/ChangeLog
2013-11-25  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        * c-objc-common.h (LANG_HOOKS_CILKPLUS_CILK_SPEC_COPY_BODY): New
        define.

gcc/ChangeLog
2013-11-25  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        * langhooks.h (lang_hooks_for_cilkplus::cilk_specific_copy_tree_body):
        New field.
        * langhooks-def.h
        (LANG_HOOKS_CILKPLUS::LANG_HOOKS_CILKPLUS_CILK_SPEC_COPY_BODY):
        Likewise.
        (LANG_HOOKS_CILKPLUS_CILK_SPEC_COPY_BODY): New #define.

gcc/cp/ChangeLog
2013-11-25  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        * cp-tree.h (cilk_valid_spawn): New prototype.
        (gimplify_cilk_spawn): Likewise.
        (cp_cilk_install_body_wframe_cleanup): Likewise.
        (cilk_create_lambda_fn_tmp_var): Likewise.
        (create_try_catch_expr): Likewise.
        * decl.c (finish_function): Insert Cilk function-calls when a
        _Cilk_spawn is used in a function.
        * parser.c (cp_parser_postfix_expression): Added RID_CILK_SPAWN and
        RID_CILK_SYNC cases.
        * cp-cilkplus.c (set_cilk_except_flag): New function.
        (set_cilk_except_data): Likewise.
        (cp_cilk_install_body_wframe_cleanup): Likewise.
        (is_lambda_function): Likewise.
        (cp_cilk_copy_tree_body_r): Likewise.
        * except.c (create_try_catch_expr): Likewise.
        * parser.h (IN_CILK_SPAWN): New #define.
        * cp-objcp-common.h (LANG_HOOKS_CILKPLUS_GIMPLIFY_SPAWN): Likewise.
        * cp-objcp-common.h (LANG_HOOKS_CILKPLUS_CILK_SPEC_COPY_BODY):
        Likewise.
        (LANG_HOOKS_CILKPLUS_DETECT_SPAWN_AND_UNWRAP): Likewise.
        (LANG_HOOKS_CILKPLUS_FRAME_CLEANUP): Likewise.
        * pt.c (tsubst_expr): Added CILK_SPAWN_STMT and CILK_SYNC_STMT cases.
        * semantics.c (potential_constant_expression_1): Likewise.
        * typeck.c (cp_build_compound_expr): Reject a spawned function in a
        compound expression.
        (check_return_expr): Reject a spawned function in a return expression.

gcc/testsuite/ChangeLog
2013-11-25  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        * g++.dg/cilk-plus/CK/catch_exc.cc: New test case.
        * g++.dg/cilk-plus/CK/const_spawn.cc: Likewise.
        * g++.dg/cilk-plus/CK/fib-opr-overload.cc: Likewise.
        * g++.dg/cilk-plus/CK/fib-tplt.cc: Likewise.
        * g++.dg/cilk-plus/CK/lambda_spawns.cc: Likewise.
        * g++.dg/cilk-plus/CK/lambda_spawns_tplt.cc: Likewise.
        * g++.dg/cilk-plus/cilk-plus.exp: Added support to run Cilk Keywords
        test stored in c-c++-common.  Also, added the Cilk runtime's library
        to the ld_library_path.

So, is this patch Ok to install?

Thanks,

Balaji V. Iyer.

> Jason

_Cilk_spawn and _Cilk_sync for C++

Commit Message

Comments

Patch