[PATCH] Cilk Keywords (_Cilk_spawn and _Cilk_sync) for C (and C++)

[Jeff Law]

On 09/11/13 12:18, Iyer, Balaji V wrote:
> Hello Everyone, Couple weeks back, I had submitted a patch for review
> that will implement Cilk keywords (_Cilk_spawn and _Cilk_sync) into
> the C compiler. I recently finished C++ implementation also. In this
> email, I am attaching 2 patches: 1 for C (and the common parts for C
> and C++) and 1 for C++. The C++ Changelog is labelled
> cp-ChangeLog.cilkplus and the other one is just ChangeLog.cilkplus.
> There isn't much changes in the C patch. Only noticeable changes
> would be moving functions to the common parts so that C++ can use
> them.
>
> It passes all the tests and does not affect  (by affect I mean fail a
> passing test or pass a failing one) any of the other tests in the
> testsuite directory.
>
> Is this Ok for trunk?
Here are my final notes from this pass over the changes.  If you could 
send an updated patch to the list, it would be appreciated.  I may have 
more comments now that I've looked over everything and have a slightly 
better understanding of the overall structure.


I'm not a C++ guy, but are you going to have to do anything special with 
lambdas?  Such as in cilk_set_spawn_marker?

I'm going to assume you don't need to do anything for the C++ specific 
decls in copy_decl_for_cilk.  Note I didn't look at any of the C++ 
specific files, so if you're handling it there, that's fine with me.

I didn't look at the tests closely.  How thorough are those tests, 
particularly for front-end issues?  The reason I ask is those tests will 
help ensure that folks don't break the cilk+ support as often as they 
might otherwise.  Basically they cover for the lack of knowledge about 
the cilk+ implementation by providing developers a heads-up that they 
broke something.  Are there any internal testsuites Intel could 
contribute to help beef up testing?

Can you #undef the helper macros SUBTREE, MODIFIED, INITIALIZED that are 
defined in extract_free_variables?  A nit, but those kind of defines can 
certainly surprised folks.  Actually, unless there's a compelling 
reason, why not just go ahead and make the calls to 
extract_free_variables explicit and drop the macros completely?

In all, I didn't see anything that made me say "wait, this is a huge 
issue we need to address".  Presumably you and Aldy worked through those 
before I got involved ;-)

jeff