[Richard, small question for you below].

Not all your changes to Makefile.in have a changelog entry.

+c-family/cilk.o : c-family/cilk.c $(TREE_H) $(SYSTEM_H) $(CONFIG_H)
toplev.h \
+        $(TREE_H) coretypes.h tree-iterator.h $(TREE_INLINE_H)
$(CGRAPH_H) \
+               $(DIAGNOSTIC_CORE_H) $(GIMPLE_H) $(CILK_H)

cilk.c includes langhooks.h and c-family/c-common.h, but I don't see dependences for them.

+  if (flag_enable_cilkplus)

We really should do a mass renaming of this flag. It should be flag_cilkplus. Not necessary to get this patch in, but highly desirable, or perhaps as a separate patch.

+  /* IF the function has _Cilk_spawn in front of a function call
inside it

Lowercase "IF". Also, this sentence reads funny. I don't quite understand it. "function call inside it"? Can you reword it?


+/* Creates the internal functions for spawn helper and parent.  */
+
+void
+c_install_body_with_frame_cleanup (tree fndecl, tree body)
+{

Since this is in the global namespace, it needs a Cilk specific name. Perhaps cilk_install_body_with_frame_cleanup, or something to that effect?

+  tree list;
+  tree frame = make_cilk_frame (fndecl);
+  tree dtor = build_cilk_function_exit (frame, false, false);
+  add_local_decl (cfun, frame);
+
+  DECL_SAVED_TREE (fndecl) = (list = alloc_stmt_list ());

Split this into two statements.

+             error_at (input_location, "consecutive _Cilk_spawn keywords not "
+                       "permitted");

"are not permitted"

+
+/* Marks CALL, a CALL_EXPR, as a spawned function call.  */
+
+tree
+c_build_spawns (location_t loc, tree call)
+{
+  cilkplus_set_spawn_marker (loc, call);
+  tree spawn_stmt = build1 (CILK_SPAWN_STMT, TREE_TYPE (call), call);
+  TREE_SIDE_EFFECTS (spawn_stmt) = 1;
+  return spawn_stmt;
+}

First, why the plural?  Second, name should probably be c_build_cilk_spawn

+
+/* Returns a tree of type CILK_SYNC_STMT if Cilk Plus is enabled. Otherwise
+   return error_mark_node.  */

Two spaces after sentence.

+
+tree
+c_build_sync (location_t loc)
+{
+  if (!flag_enable_cilkplus)
+    {
+      error_at (loc, "-fcilkplus not enabled");
+      return error_mark_node;
+    }

If actually needed, the check for cilkplus should be done in the caller.

Also, rename to c_build_cilk_sync

+  if (flag_enable_cilkplus)
+    cpp_define_formatted (pfile, "__cilk=%d", 200);

Why the %d?  Can't you just hard code the 200 into the string?

+@item CILK_SPAWN_STMT
+
+Used to represent a spawning function in the Cilk Plus language extension. This
+tree has one field that holds the name of the spawning function.
+_Cilk_spawn can be written in C in the following way:

First, two spaces after "extension.".

Second, you should provide an accessor macro for the operand. For example for a TRANSACTION_EXPR, we have the following in tree.h:

/* TM directives and accessors.  */
#define TRANSACTION_EXPR_BODY(NODE) \
  TREE_OPERAND (TRANSACTION_EXPR_CHECK (NODE), 0)

After you do this, all uses of TREE_OPERAND (blah, 0) should use this new accessor, and the documentation here should match.

+ _Cilk_spawn keyword is parsed and the function that it contains is marked as

The _Cilk_spawn keyword...

s/that it/it

+a spawning function.  The spawning function is called the spawner.  At the end
+of the parsing phase, appropriate internal (builtin) functions are added to

Rename "internal (builtin) functions" to just "built-in functions", and by the way, the correct nomenclature in GCC is "built-in".

+the spawner that are defined in Cilk runtime.  The appropriate locations of

s/in Cilk/in the Cilk/

+these functions, and the internal structures are detailed in
+@code{cilk_init_builtins} in file @file{cilk-common.c}.  The pointers to Cilk

s/in file/in the file/


+functions and fields of internal structures are described in @file{cilk.h}.
+The builtin functions are described in @file{cilk-builtins.def}.

built-in

+
+During the gimplification stage, a new "spawn-helper" function is created.

You can probably just say "during gimplification, ..."

+The spawned function is replaced with a spawn helper function in the spawner.
+The spawned function-call is moved into the spawn helper.  The main function
+that does these transformations is @code{gimplify_cilk_spawn} in
+@file{c-family/cilk.c}.  In the spawn-helper, the gimplification function
+@code{gimplify_call_expr}, inserts a function call @code{__cilkrts_detach}.
+This function is expanded by @code{builtin_expand_cilk_detach} located in
+@file{c-family/cilk.c}.
+
+@item _Cilk_sync:
+_Cilk_sync is parsed like any regular keyword.  During gimplification stage,

s/any regular/a
s/During gimplification stage/During gimplification/

+the function @code{gimplify_cilk_sync} in @file{c-family/cilk.c}, will replace
+this keyword with a set of functions that are stored in Cilk Runtime.  One of

"in the Cilk run-time

+the internal functions inserted during gimplification stage,

s/gimplification stage/gimplification

--- gcc/expr.c
+++ gcc/expr.c
@@ -9569,6 +9569,21 @@ expand_expr_real_1 (tree exp, rtx target, enum 
machine_mode tmode,
        }

       return expand_constructor (exp, target, modifier, false);
+    case INDIRECT_REF:
+      {
+       tree exp1 = TREE_OPERAND (exp, 0);
+       if (modifier != EXPAND_WRITE)
+         {
+           tree t = fold_read_from_constant_string (exp);
+           if (t)
+             return expand_expr (t, target, tmode, modifier);
+         }
+       op0 = expand_expr (exp1, NULL_RTX, VOIDmode, EXPAND_SUM);
+       op0 = memory_address (mode, op0);
+       temp = gen_rtx_MEM (mode, op0);
+       set_mem_attributes (temp, exp, 0);
+       return temp;
+      }

Ughhh, what's the rationale for this? Are generic changes to expand_expr really needed?

+  /* During LTO, the is_cilk_function flag gets cleared.
+     If __cilkrts_pop_frame is called, then this definitely must be a
+     cilk function.  */
+  if (cfun)
+    cfun->is_cilk_function = 1;

I don't know much about our LTO implementation, but perhaps you need to teach LTO to stream this bit in/out? And of course, an accompanying LTO test to check for this problem you're encountering would be appropriate.

+       /* We need frame pointer for all Cilk Plus functions that uses
+         Cilk Keywords.  */
+       || (flag_enable_cilkplus && cfun->is_cilk_function)

"need a frame pointer"

"that use"

s/Keywords/keywords/

+  /* This variable will tell whether we are on a spawn helper or not */
+  unsigned int is_cilk_helper_function : 1;

Where is this used?

+  /* Nonzero if this is a Cilk function that spawns. */
+  unsigned int calls_spawn : 1;

Global name space again. use calls_cilk_spawn. Using just "calls_spawn" is bound to be ambiguous and interfere with either current usage of spawn or future usage not related to Cilk.


--- gcc/ipa-inline-analysis.c
+++ gcc/ipa-inline-analysis.c
@@ -1433,6 +1433,9 @@ initialize_inline_failed (struct cgraph_edge *e)
     e->inline_failed = CIF_REDEFINED_EXTERN_INLINE;
   else if (e->call_stmt_cannot_inline_p)
     e->inline_failed = CIF_MISMATCHED_ARGUMENTS;
+  else if (flag_enable_cilkplus && cfun && cfun->calls_spawn)
+    /* We can't inline if the function is spawing a function.  */
+    e->inline_failed = CIF_BODY_NOT_AVAILABLE;

Hmmm, if we don't have a cfun, perhaps we should be sticking this calls_spawn bit in the cgraph node.

Richard?  Anyone?

@@ -439,6 +440,8 @@ struct GTY(()) tree_base {
   unsigned protected_flag : 1;
   unsigned deprecated_flag : 1;
   unsigned default_def_flag : 1;
+  unsigned is_cilk_spawn : 1;
+  unsigned is_cilk_helper_fn : 1;

Just curious, do you need these bits both in the tree decl *and* in cfun?

And BTW, where is is_cilk_helper_fn used? This doesn't seem to be used anywhere.

+/* True if the function call is a spawned call.  */
+#define SPAWN_CALL_P(N) ((N)->base.is_cilk_spawn)

Global namespace again.  Perhaps CILK_SPAWN_CALL_P?

+
+/* True if the function is a cilk helper function or something that cilk
+   touches.  */
+#define CILK_FN_P(N) (N->base.is_cilk_helper_fn)

Similarly here.  Unused.

+
+/* True if this call is the point at which a wrapper should detach. */
+#define SPAWN_DETACH_POINT(NODE) ((NODE)->base.default_def_flag)
+

If you are going to overload default_def_flag, you need to document this in tree.h, around here:

   default_def_flag:

       TYPE_VECTOR_OPAQUE in
           VECTOR_TYPE

       SSA_NAME_IS_DEFAULT_DEF in
           SSA_NAME

       DECL_NONLOCAL_FRAME in
           VAR_DECL

Oh, global namespace again.   CILK_SPAWN_DETACH_POINT?

@@ -3496,7 +3510,7 @@ struct GTY(()) tree_function_decl {
      ???  The bitfield needs to be able to hold all target function
          codes as well.  */
   ENUM_BITFIELD(built_in_function) function_code : 11;
-  ENUM_BITFIELD(built_in_class) built_in_class : 2;
+  ENUM_BITFIELD(built_in_class) built_in_class : 3;

What's this for?

-  struct pointer_map_t *debug_map;
+  struct pointer_map_t *debug_map;

Unnecessary whitespace change.

+
+  /* Cilk keywords currently needs to replace some variables that
+     ordinary nested functions do not. */
+  bool remap_var_for_cilk;

s/needs/need

and first line has two useless spaces

+  if (DECL_BUILT_IN_CLASS (exp) == BUILT_IN_NORMAL)
+    return DECL_FUNCTION_CODE (exp) == BUILT_IN_MEMCPY;
+  return lang_hooks.cilkplus.spawnable_constructor (exp);
+  return false;

Unreachable code.

+#define LANG_HOOKS_CILKPLUS_SPAWNABLE_CTOR hook_bool_tree_false
+#define LANG_HOOKS_CILKPLUS_RECOGNIZE_SPAWN hook_bool_tree_false

Are these two hooks ever set to anything but hook_bool_tree_false? If so, why the need for them?

+      if (!frame)
+       {
+         error_at (input_location, "spawning function lacks frame descriptor");
+         frame = null_pointer_node;
+       }
+      else
+       frame = build1 (ADDR_EXPR, build_pointer_type (TREE_TYPE (frame)),
+                       frame);

This is a personal preference, so feel free to ignore me, but I prefer the true condition to come first when reading code:

if (frame)
  blah
else
  blah blah

Also, why not use "loc" instead of input_location? Unless loc does not apply.

+         /* If there are errors, then there is no point in expanding the

s/then there/there

+/* Language hooks related to Cilk Plus.  */
+
+struct lang_hooks_for_cilkplus
+{
+  /* Returns true if the constructor in C++ is spawnable.  Default is false.  
*/
+  bool (*spawnable_constructor) (tree);
+
+  /* Returns true if it is able to recognize spawned function call inside the
+     language-dependent trees (mainly used for C++).  */
+  bool (*recognize_spawn) (tree);

s/recognize spawned/recognize a spawned/

+
+  /* Returns true if the call expr passed in is a spawned function call.  */
+  bool (*cilk_valid_spawn) (tree *);

s/in//

+
+  /* Function to add the clean up functions after spawn.  The reason why it is
+     language dependent is because in C++, it must handle exceptions.  */
+  void (*install_body_with_frame_cleanup) (tree, tree);
+
+  /* Function to gimplify a spawned function call.  Returns enum gimplify
+     status, but as mentioned in previous comment, we can't see that type here,
+     so just return an int.  */

s/in previous comment/in a previous comment/

+  int (*gimplify_cilk_spawn) (tree *, gimple_seq *,
+                             gimple_seq *);
+
+  /* Function to gimplify _Cilk_sync. Same rationale as above for returning
+     int.  */

Two spaces before 'Same'.

--- gcc/lto/lto-lang.c
+++ gcc/lto/lto-lang.c
@@ -35,6 +35,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "diagnostic-core.h"
 #include "toplev.h"
 #include "lto-streamer.h"
+#include "cilk.h"


Included file is not listed as dependency in makefile.

+tree cilk_trees[(int) CILK_TI_MAX];
+HOST_WIDE_INT cilk_wrapper_count;

Comments for these would be nice.

Just a suggestion, but could all uses of cilk_wrapper_count happen in the same file, thus avoiding having to make this a non-static global?

+
+/* Returns the value in structure FRAME pointed by the FIELD_NUMBER
+   (e.g. X.y).  */
+
+tree
+dot (tree frame, int field_number, bool volatil)
+{
+  tree field = cilk_trees[field_number];
+  field = fold_build3 (COMPONENT_REF, TREE_TYPE (field), frame, field,
+                      NULL_TREE);
+  TREE_THIS_VOLATILE (field) = volatil;
+  return field;
+}

Globally visible symbol. You need to use something like cilk_dot. Also, please document what FIELD_NUMBER is-- presumably this is an index into something else? Furthermore, please document what tree type you expect for FRAME.

+
+/* Returns the address of a field in FRAME_PTR, pointed by FIELD_NUMBER.
+   (e.g. (&X)->y).  */
+
+tree
+arrow (tree frame_ptr, int field_number, bool volatil)
+{

The same things apply here as well.

+/* This function will add FIELD of type TYPE to a defined builtin structure.  
*/

built-in

+/* This function will define a builtin function of NAME, of type FNTYPE and
+   register it under the builtin function code CODE.  */

built-in everywhere. For that matter, just do a global search and replace. I won't mention it again.

+/* Creates and Initializes all the builtin Cilk keywords functions and three

initializes

+  /* Now add them to a common structure, whose fields are #defined to something

s/,//

+     struct __cilkrts_worker {
+       __cilkrts_stack_frame *volatile *volatile tail;
+       __cilkrts_stack_frame *volatile *volatile head;
+       __cilkrts_stack_frame *volatile *volatile exc;
+       __cilkrts_stack_frame *volatile *volatile protected_tail;
+       __cilkrts_stack_frame *volatile *ltq_limit;

What's this "*volatile *volatile" stuff?

+
+  /* void __cilkrts_enter_frame (__cilkrts_stack_frame *);  */
+  cilk_enter_fndecl = install_builtin ("__cilkrts_enter_frame_1", fptr_fun,
+                                      BUILT_IN_CILK_ENTER_FRAME, false);
+
+  /* void __cilkrts_enter_frame_fast (__cilkrts_stack_frame *);  */
+  cilk_enter_fast_fndecl =
+    install_builtin ("__cilkrts_enter_frame_fast_1", fptr_fun,
+                    BUILT_IN_CILK_ENTER_FRAME_FAST, false);
+
+  /* void __cilkrts_pop_frame (__cilkrts_stack_frame *);  */
+  cilk_pop_fndecl = install_builtin ("__cilkrts_pop_frame", fptr_fun,
+                                    BUILT_IN_CILK_POP_FRAME, false);
etc
etc

BTW, aren't there generic ways of building built-ins? Surely there must be a way to adapt it to even use __cilkrts_stack_frame. But I could be wrong...

+  /* Initialize wrapper function count to zero.  */
+  cilk_wrapper_count = 0;

Do not document the obvious.

+  /* If it is passed in as an address, then just use the value directly
+     since the function in inlined.  */

is inlined

+/* Expands the __cilkrts_pop_frame function call stored in EXP.
+   Returns const0_rtx.  */
+
+rtx
+expand_builtin_cilk_pop_frame (tree exp)

If you always return the same thing, isn't that redundant? Also, I see the caller doesn't even use return value.

+/* Expands the cilk_detach function call stored in EXP.  Returns const0_rtx.  
*/
+
+rtx
+expand_builtin_cilk_detach (tree exp)

Similarly to above.  Return value redundant.

+enum cilk_block_type {
+    CILK_BLOCK_SPAWN = 30, /* Indicates a Cilk Spawn block.   */
+    CILK_BLOCK_FOR        /* Indicates _Cilk_for statement block.  */

_Cilk_spawn?  Be consistent between both.

+/* Marks the CALL_EXPR, FCALL, as a Spawned function call and the current

spawned

+   function as a spawner.  Emits error if the function call is outside a

Emit

+   function or if a non function-call is spawned.  */
+
+void
+cilkplus_set_spawn_marker (location_t loc, tree fcall)
+{
+  if (!current_function_decl)
+    error_at (loc, "Cilk spawn may only be used inside a function");

I've seen errors with "Cilk spawn" and others with "_Cilk_spawn". Which is correct?

+/* Gimplifies the cilk_sync expression passed in *EXPR_P.  Returns GS_ALL_DONE
+   when finished.  */
+
+int
+gimplify_cilk_sync (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p
+                   ATTRIBUTE_UNUSED)
+{
+  tree sync_expr = build_cilk_sync ();
+  *expr_p = NULL_TREE;
+  gimplify_and_add (sync_expr, pre_p);
+  return GS_ALL_DONE;

I'm not a big fan of functions that always return the same thing. The caller should set GS_ALL_DONE accordingly.

+/* Trying to get the correct cfun for the FUNCTION_DECL indicated by OUTER.  */
+
+static void
+pop_cfun_to (tree outer)
+{
+  pop_cfun ();
+  current_function_decl = outer;
+  gcc_assert (cfun == DECL_STRUCT_FUNCTION (current_function_decl));
+  gcc_assert (cfun->decl == current_function_decl);
+}
+
+/* This function does whatever is necessary to make the compiler emit a newly
+   generated function, FNDECL.  */
+
+static void
+call_graph_add_fn (tree fndecl)
+{
+  const tree outer = current_function_decl;
+  struct function *f = DECL_STRUCT_FUNCTION (fndecl);
+
+  gcc_assert (TREE_CODE (fndecl) == FUNCTION_DECL);
+
+  f->is_cilk_function = 1;
+  f->is_cilk_helper_function = 1;
+
+  f->curr_properties = cfun->curr_properties;
+  gcc_assert (cfun == DECL_STRUCT_FUNCTION (outer));
+  gcc_assert (cfun->decl == outer);
+
+  push_cfun (f);
+
+  cgraph_add_new_function (fndecl, false);
+
+  /* Calling cgraph_finalize_function now seems to be the only way to
+     prevent a crash due to cgraph becoming confused over whether the
+     function is needed.  */
+  cgraph_finalize_function (fndecl, true);

What's the problem here?  It looks like you're papering over a problem.

+
+  pop_cfun_to (outer);
+}
+
+/* Return true if this is a tree which is allowed to contain a spawn as
+   operand 0.
+   A spawn call may be wrapped in a series of unary operations such
+   as conversions.  These conversions need not be "useless"
+   to be disregarded because they are retained in the spawned
+   statement.  They are bypassed only to look for a spawn
+   within.
+   A comparison to constant is simple enough to allow, and
+   is used to convert to bool.  */
+
+static bool
+cilk_ignorable_spawn_rhs_op (tree exp)
+{
+  enum tree_code code = TREE_CODE (exp);
+  switch (TREE_CODE_CLASS (code))
+    {
+    case tcc_expression:
+      return code == ADDR_EXPR;
+    case tcc_comparison:
+      /* We need the spawn as operand 0 for now.   That's where it
+        appears in the only case we really care about, conversion
+        to bool. */

Two spaces after period.

+      return (TREE_CODE (TREE_OPERAND (exp, 1)) == INTEGER_CST);
+    case tcc_unary:
+    case tcc_reference:
+      return true;
+    default:
+      return false;
+    }
+}
+
+/* Helper function for walk_tree.  If *TP is a CILK_SPAWN_STMT, then unwrap
+   this "wrapper" and  *WALK_SUBTREES is set to 0.  The function returns
+   NULL_TREE regardless.  */
+
+static tree
+unwrap_cilk_sync_stmt (tree *tp, int *walk_subtrees, void *)
+{
+  if (TREE_CODE (*tp) == CILK_SPAWN_STMT)
+    {
+      /* Clear the SPAWN_CALL flag to avoid multiple spawn runnings.  */
+      if (SPAWN_CALL_P (*tp))
+       SPAWN_CALL_P (*tp) = 0;
+      *tp = TREE_OPERAND (*tp, 0);
+      *walk_subtrees = 0;
+    }
+  return NULL_TREE;
+}
+
+/* This function checks to see if the constructor, EXP can be spawnable.  */

the constructor in EXP

+
+static bool
+cilk_spawnable_constructor (tree exp)
+{
+  if (TREE_CODE (exp) != ADDR_EXPR)
+    return false;
+  exp = TREE_OPERAND (exp, 0);
+  if (TREE_CODE (exp) != FUNCTION_DECL)
+    return false;
+  if (DECL_BUILT_IN_CLASS (exp) == BUILT_IN_NORMAL)
+    return DECL_FUNCTION_CODE (exp) == BUILT_IN_MEMCPY;
+  return lang_hooks.cilkplus.spawnable_constructor (exp);
+  return false;

Unreachable return.

+}
+
+/* Returns true when EXP is a CALL_EXPR with _Cilk_spawn in front.  Unwraps

Extra space at end of "Unwraps".

+   CILK_SPAWN_STMT wrapper from the CALL_EXPR in *EXP0 statement.  */
+
+static bool
+recognize_spawn (tree exp, tree *exp0)
+{
+  if (TREE_CODE (exp) == CILK_SPAWN_STMT)
+    {
+      /* Remove the CALL_EXPR from CILK_SPAWN_STMT wrapper and return true.  */
+      exp = TREE_OPERAND (exp, 0);
+      walk_tree (exp0, unwrap_cilk_sync_stmt, NULL, NULL);
+    }
+  else
+    {
+      if (TREE_CODE (exp) != CALL_EXPR && TREE_CODE (exp) != TARGET_EXPR)
+       return lang_hooks.cilkplus.recognize_spawn (exp);
+      if (!SPAWN_CALL_P (exp))
+       return false;
+    }
+  SPAWN_CALL_P (exp) = 0;
+
+  if (TREE_CODE (exp) == CALL_EXPR)
+    SPAWN_DETACH_POINT (exp) = 1;
+  else if (TREE_CODE (exp) == TARGET_EXPR && TARGET_EXPR_INITIAL (exp))
+    SPAWN_DETACH_POINT (TARGET_EXPR_INITIAL (exp)) = 1;
+  return true;
+}
+
+/* Returns true if *EXP0 is a recognized form of spawn.  Recognized forms are,
+   after conversion to void, a call expression at outer level or an assignment
+   at outer level with the right hand side being a spawned call.
+   Note that `=' in C++ may turn into a CALL_EXPR rather than a MODIFY_EXPR.
+
+   If this function returns true it has cleared the SPAWN_CALL_P or

returns true, it has

+   AGGR_INIT_VIA_SPAWN_P flag on the call to which the spawn keyword was

extra space at end

+   attached and set the SPAWN_DETACH_POINT flag instead.  */
+
+bool
+cilk_valid_spawn (tree *exp0)
+{
+  tree exp = *exp0;
+  bool warn;
+
+  if (!TREE_SIDE_EFFECTS (exp))
+    return false;
+
+  /* If the function contains no Cilk code, this isn't a spawn.  */
+  if (!cfun->cilk_frame_decl)
+    return false;
+
+  /* Strip off any conversion to void.  It does not affect whether spawn

Extra space at end. Can you check runaway white space at the end of lines in all of your patch?

+     is supported here.  */
+  if (TREE_CODE (exp) == CONVERT_EXPR && VOID_TYPE_P (TREE_TYPE (exp)))
+    exp = TREE_OPERAND (exp, 0);
+
+  if (TREE_CODE (exp) == MODIFY_EXPR || TREE_CODE (exp) == INIT_EXPR)
+    exp = TREE_OPERAND (exp, 1);
+
+  while (cilk_ignorable_spawn_rhs_op (exp))
+    exp = TREE_OPERAND (exp, 0);
+
+  if (TREE_CODE (exp) == TARGET_EXPR)
+    if (TARGET_EXPR_INITIAL (exp)
+       && TREE_CODE (TARGET_EXPR_INITIAL (exp)) != AGGR_INIT_EXPR)
+      exp = TARGET_EXPR_INITIAL (exp);
+
+  if (exp == NULL_TREE)
+    return false; /* Happens with C++ TARGET_EXPR.  */

It is customary for comments to go in a line by themselves, before the code. Common exceptions are field comments for structures and such.

+
+  while (TREE_CODE (exp) == CLEANUP_POINT_EXPR || TREE_CODE (exp) == EXPR_STMT)
+    exp = TREE_OPERAND (exp, 0);
+
+  /* Now we have a call, or this isn't a valid spawn. */

Two spaces after period.

+  /* This will reject any outer non-spawn AGGR_INIT_EXPR
+     that is valid because of a spawn inside.  */
+  if (recognize_spawn (exp, exp0))
+    return true;
+
+  if (TREE_CODE (exp) != CALL_EXPR)
+    return false;
+
+  /* This may be a call that is not a spawn itself but contains a spawn.
+     In that case the call should be a constructor.

that case, the call

+
+     x = spawn f();
+
+     may expand to
+
+     (call operator= (&var1, (convert &(target var2 (aggr_init/spawn ...))))
+
+     operator= may be a function or a call to __builtin_memcpy (which
+     will have one more argument, the size).
+
+     What we specifically support is the address of the value
+     initialized by a spawning AGGR_INIT_EXPR being passed as
+     the second argument to a function.
+
+     Maybe we should ensure that the function is a constructor
+     or builtin memcpy?
+  */
+
+  warn = !cilk_spawnable_constructor (CALL_EXPR_FN (exp));
+
+  /* The function address of a call may not be computed via a spawn.
+     Look at the arglist only, and only the second argument which
+     is the RHS of any plausible assignment or copy.  The first
+     argument is the LHS.  A third argument could be a size for
+     memcpy.  This path supports op= in addition to =, only because
+     it is easy to do so. */
+  if (call_expr_nargs (exp) < 2)
+    return false;
+
+  exp = CALL_EXPR_ARG (exp, 0);
+
+  STRIP_USELESS_TYPE_CONVERSION (exp);
+
+  if (TREE_CODE (exp) == ADDR_EXPR)
+    exp = TREE_OPERAND (exp, 0);
+
+  if (TREE_CODE (exp) == TARGET_EXPR)
+    exp = TARGET_EXPR_INITIAL (exp);
+
+  if (!exp || !recognize_spawn (exp, exp0))
+    return false;
+
+  if (warn)
+    warning (0, "suspicious use of _Cilk_spawn");
+  return true;
+}
+
+/* This function will return a FNDECL using information from *WD.  */
+
+static tree
+build_cilk_helper_decl (struct wrapper_data *wd)
+{
+  char name[20];
+  if (wd->type == CILK_BLOCK_FOR)
+    sprintf (name, "_cilk_for_%ld", cilk_wrapper_count++);
+  else if (wd->type == CILK_BLOCK_SPAWN)
+    sprintf (name, "_cilk_spn_%ld", cilk_wrapper_count++);
+  else
+    gcc_unreachable ();
+
+  clean_symbol_name (name);
+  tree fndecl = build_decl (UNKNOWN_LOCATION, FUNCTION_DECL,
+                           get_identifier (name), wd->fntype);
+
+  TREE_PUBLIC (fndecl) = 0;
+  TREE_STATIC (fndecl) = 1;
+  TREE_USED (fndecl) = 1;
+  DECL_ARTIFICIAL (fndecl) = 0;
+  DECL_IGNORED_P (fndecl) = 0;
+  DECL_EXTERNAL (fndecl) = 0;
+
+  if (wd->nested)
+    DECL_CONTEXT (fndecl) = wd->context;
+  else
+    /* In C++, copying the outer function's context makes the loop
+       function appear like a static member function.  */
+    DECL_CONTEXT (fndecl) = DECL_CONTEXT (wd->context);
+
+  tree block = make_node (BLOCK);
+  DECL_INITIAL (fndecl) = block;
+  TREE_USED (block) = 1;
+  gcc_assert (!DECL_SAVED_TREE (fndecl));
+
+  /* Inlining would defeat the purpose of this wrapper.
+     Either it secretly switches stack frames or it allocates
+     a stable stack frame to hold function arguments even if
+     the parent stack frame is stolen.  */
+  DECL_UNINLINABLE (fndecl) = 1;
+
+  tree result_decl = build_decl (UNKNOWN_LOCATION, RESULT_DECL, NULL_TREE,
+                                void_type_node);
+  DECL_ARTIFICIAL (result_decl) = 0;
+  DECL_IGNORED_P (result_decl) = 1;
+  DECL_CONTEXT (result_decl) = fndecl;
+  DECL_RESULT (fndecl) = result_decl;
+
+  return fndecl;
+}
+
+/* A function used by walk tree to find wrapper parms.  */
+
+static bool
+wrapper_parm_cb (const void *key0, void **val0, void *data)
+{
+  struct wrapper_data *wd = (struct wrapper_data *)data;
+  tree arg = * (tree *)&key0;
+  tree val = (tree)*val0;
+  tree parm;
+
+  if (val == error_mark_node || val == arg)
+    return true;
+
+  if (TREE_CODE (val) == PAREN_EXPR)
+    {
+      /* We should not reach here with a register receiver.
+        We may see a register variable modified in the
+        argument list.  Because register variables are
+        worker-local we don't need to work hard to support
+        them in code that spawns. */
+      if ((TREE_CODE (arg) == VAR_DECL) && DECL_HARD_REGISTER (arg))
+       {
+         error_at (EXPR_LOCATION (arg),
+                   "explicit register variable %qD may not be modified in "
+                   "spawn", arg);
+         arg = null_pointer_node;
+       }
+      else
+       arg = build1 (ADDR_EXPR, build_pointer_type (TREE_TYPE (arg)), arg);
+       
+      val = TREE_OPERAND (val, 0);
+      *val0 = val;
+      gcc_assert (TREE_CODE (val) == INDIRECT_REF);
+      parm = TREE_OPERAND (val, 0);
+      STRIP_NOPS (parm);
+    }
+  else
+    parm = val;
+  TREE_CHAIN (parm) = wd->parms;
+  wd->parms = parm;
+  wd->argtypes = tree_cons (NULL_TREE, TREE_TYPE (parm), wd->argtypes);
+  wd->arglist = tree_cons (NULL_TREE, arg, wd->arglist);
+  return true;
+}
+
+/* This function is used to build a wrapper of a certain type.  */
+
+static void
+build_wrapper_type (struct wrapper_data *wd)
+{
+  wd->arglist = NULL_TREE;
+  wd->parms = NULL_TREE;
+  wd->argtypes = void_list_node;
+
+  pointer_map_traverse (wd->decl_map, wrapper_parm_cb, wd);
+  gcc_assert (wd->type != CILK_BLOCK_FOR);
+
+  /* Now build a function.
+     Its return type is void (all side effects are via explicit parameters).
+     Its parameters are WRAPPER_PARMS with type WRAPPER_TYPES.
+     Actual arguments in the caller are WRAPPER_ARGS.  */
+  wd->fntype = build_function_type (void_type_node, wd->argtypes);
+}
+
+/* This function checks all the CALL_EXPRs in *TP found by cilk_outline.  */
+
+static tree
+check_outlined_calls (tree *tp, int *walk_subtrees ATTRIBUTE_UNUSED,
+                     void *data)
+{
+  bool *throws = (bool *)data;
+  tree t = *tp;
+  int flags;
+
+  if (TREE_CODE (t) != CALL_EXPR)
+    return 0;
+  flags = call_expr_flags (t);
+
+  if (!(flags & ECF_NOTHROW) && flag_exceptions)
+    *throws = true;
+  if (flags & ECF_RETURNS_TWICE)
+    error_at (EXPR_LOCATION (t),
+             "can not spawn call to function that returns twice");

cannot is one word

"to a function that returns"...

+  return 0;
+}
+
+/* Each DECL in the source code (spawned statement) is passed to this function
+   once.  Each instance of the DECL is replaced with the result of this
+   function.
+
+   The parameters of the wrapper should have been entered into the map already.
+   This function only deals with variables with scope limited to the
+   spawned expression.  */
+
+static tree
+copy_decl_for_cilk (tree decl, copy_body_data *id)
+{
+  switch (TREE_CODE (decl))
+    {
+    case VAR_DECL:
+      return copy_decl_no_change (decl, id);
+
+    case LABEL_DECL:
+      error_at (EXPR_LOCATION (decl), "invalid use of label %q+D in spawn",
+               decl);
+      return error_mark_node;
+
+    case RESULT_DECL:
+      /* PARM_DECL has already been entered into the map.  */

You mean RESULT_DECL has already been entered?

+    case PARM_DECL:
+      /* PARM_DECL has already been entered into the map.  */

But you can probably combine the comments into one "DECL has already been entered..".

+
+  id.src_fn = outer_fn; /* Copy FROM the function containing the spawn...  */
+  id.dst_fn = inner_fn; /* ...TO the wrapper.  */
+  id.src_cfun = DECL_STRUCT_FUNCTION (outer_fn);
+
+  id.retvar = 0; /* There shall be no RETURN in spawn.  */

Comments should go in a separate line.

+  id.decl_map = wd->decl_map;
+  id.copy_decl = nested ? copy_decl_no_change : copy_decl_for_cilk;
+  id.block = DECL_INITIAL (inner_fn);
+  id.transform_lang_insert_block = NULL;
+
+  id.transform_new_cfg = true;
+  id.transform_call_graph_edges = CB_CGE_MOVE;
+  id.remap_var_for_cilk = true;
+  id.regimplify = true; /* unused? */
+
+  insert_decl_map (&id, wd->block, DECL_INITIAL (inner_fn));
+
+  /* We don't want the private variables any more.  */
+  pointer_map_traverse (wd->decl_map, nested ? for_local_cb : wrapper_local_cb,
+                       &id);
+
+  walk_tree (stmt_p, copy_tree_body_r, &id, NULL);
+
+  /* See if this function can throw or calls something that should
+     not be spawned.  The exception part is only necessary if
+     flag_exceptions && !flag_non_call_exceptions.  */
+  throws = false ;
+  (void) walk_tree_without_duplicates (stmt_p, check_outlined_calls, &throws);
+}
+
+/* Generate the body of a wrapper function that assigns the
+   result of the expression RHS into RECEIVER.  RECEIVER must
+   be NULL if this is not a spawn -- the wrapper will return
+   a value.  If this is a spawn the wrapper will return void.  */

If this is a spawn, the ....

+/* Returns a wrapper function for a _Cilk_spawn.  */
+
+static tree
+build_cilk_wrapper (tree exp, tree *args_out)
+{
+  struct wrapper_data wd;
+  tree fndecl;
+
+  init_wd (&wd, CILK_BLOCK_SPAWN);
+
+  if (TREE_CODE (exp) == CONVERT_EXPR)
+    exp = TREE_OPERAND (exp, 0);
+
+  /* Special handling for top level INIT_EXPR.  Usually INIT_EXPR means the
+     variable is defined in the spawned expression and can be private to the
+     spawn helper.  At top level INIT_EXPR defines a variable to be initialized

A top level

+     by spawn and the variable must remain in the outer function. */
+  if (TREE_CODE (exp) == INIT_EXPR)
+    {
+      extract_free_variables (TREE_OPERAND (exp, 0), &wd, ADD_WRITE);
+      extract_free_variables (TREE_OPERAND (exp, 1), &wd, ADD_READ);
+      /* TREE_TYPE should be void.  Be defensive.  */
+      if (TREE_TYPE (exp) != void_type_node)
+       extract_free_variables (TREE_TYPE (exp), &wd, ADD_READ);
+    }
+  else
+    extract_free_variables (exp, &wd, ADD_READ);
+  pointer_map_traverse (wd.decl_map, declare_one_free_variable, &wd);
+  wd.block = TREE_BLOCK (exp);
+  if (!wd.block)
+    wd.block = DECL_INITIAL (current_function_decl);
+
+  /* Now fvars maps old variable to incoming variable.  Update

maps the old variable to the incoming

+     the expression and arguments to refer to the new names.  */
+  fndecl = build_cilk_wrapper_body (exp, &wd);
+  *args_out = wd.arglist;
+
+  free_wd (&wd);
+
+  return fndecl;
+}
+
+/* Transform *SPAWN_P, a Spawned CALL_EXPR, to gimple. *SPAWN_P can be a

a spawned CALL_EXPR

Two spaces after "."

+   CALL_EXPR, INIT_EXPR or MODIFY_EXPR.  Returns GS_OK if everything is fine,
+   and GS_UNHANDLED, otherwise.  */
+
+int
+gimplify_cilk_spawn (tree *spawn_p, gimple_seq *before ATTRIBUTE_UNUSED,
+                    gimple_seq *after ATTRIBUTE_UNUSED)
+{
+  tree expr = *spawn_p;
+  tree function, call1, call2, new_args;
+  tree ii_args = NULL_TREE;
+  int total_args = 0, ii = 0;
+  tree *arg_array;
+  tree setjmp_cond_expr = NULL_TREE;
+  tree setjmp_expr, spawn_expr, setjmp_value = NULL_TREE;
+
+  cfun->calls_spawn = 1;
+  cfun->is_cilk_function = 1;
+
+  if (!flag_enable_cilkplus)
+    {
+      sorry ("_Cilk_spawn is not implemented");
+      *spawn_p = build_empty_stmt (EXPR_LOCATION (*spawn_p));
+      return GS_UNHANDLED;
+    }

How can you ever call this function without flag_enable_cilkplus? Perhaps a gcc_assert(), if at all? If it *can* happen, then you need a test case which you don't currently have.

If you do end up removing this check, then this function always returns GS_OK, which is redundant.

+
+  /* Remove cleanup point expr and expr stmt from *spawn_p.  */

s/cleanup point expr/CLEANUP_POINT_EXPR/
s/expr stmt/EXPR_STMT/

It reads better :).

+  while (TREE_CODE (expr) == CLEANUP_POINT_EXPR
+        || TREE_CODE (expr) == EXPR_STMT)
+    expr = TREE_OPERAND (expr, 0);
+
+  new_args = NULL;
+  function = build_cilk_wrapper (expr, &new_args);
+
+  /* This should give the number of parameters.  */
+  total_args = list_length (new_args);
+  arg_array = XNEWVEC (tree, total_args);
+
+  ii_args = new_args;
+  for (ii = 0; ii < total_args; ii++)
+    {
+      arg_array[ii] = TREE_VALUE (ii_args);
+      ii_args = TREE_CHAIN (ii_args);
+    }
+
+  /* A spawn wrapper has void type.  */
+  TREE_USED (function) = 1;

I'm confused by the comment.  Does the comment refer to something else?

+
+  rest_of_decl_compilation (function, 0, 0);
+
+  call1 = cilk_call_setjmp (cfun->cilk_frame_decl);
+
+  if (*arg_array == NULL_TREE)
+    call2 = build_call_expr (function, 0);
+  else
+    call2 = build_call_expr_loc_array (EXPR_LOCATION (*spawn_p), function,
+                                      total_args, arg_array);
+  *spawn_p = alloc_stmt_list ();
+  gcc_assert (cfun->cilk_frame_decl != NULL_TREE);
+
+  tree frame_ptr =
+    build1 (ADDR_EXPR, build_pointer_type (TREE_TYPE (cfun->cilk_frame_decl)),
+           cfun->cilk_frame_decl);
+  tree save_fp = build_call_expr (cilk_save_fp_fndecl, 1, frame_ptr);
+  append_to_statement_list (save_fp, spawn_p);         
+  setjmp_value = create_tmp_var (TREE_TYPE (call1), NULL);
+  setjmp_expr = fold_build2 (MODIFY_EXPR, void_type_node, setjmp_value, call1);
+
+  append_to_statement_list_force (setjmp_expr, spawn_p);
+
+  setjmp_cond_expr = fold_build2 (EQ_EXPR, TREE_TYPE (call1), setjmp_value,
+                                 build_int_cst (TREE_TYPE (call1), 0));
+  spawn_expr = fold_build3 (COND_EXPR, void_type_node, setjmp_cond_expr,
+                           call2, build_empty_stmt (EXPR_LOCATION (call1)));
+  append_to_statement_list (spawn_expr, spawn_p);
+
+  return GS_OK;
+}
+
+/* Make the frames necessary for a spawn call.  */
+
+static tree
+make_cilk_frame (tree fn)
+{
+  struct function *f = DECL_STRUCT_FUNCTION (fn);
+  tree decl;
+
+  if (f->cilk_frame_decl)
+    return f->cilk_frame_decl;
+
+  decl = build_decl (EXPR_LOCATION (fn), VAR_DECL, NULL_TREE,
+                    cilk_frame_type_decl);
+  DECL_CONTEXT (decl) = fn;
+  DECL_SEEN_IN_BIND_EXPR_P (decl) = 1;
+  f->cilk_frame_decl = decl;
+  return decl;
+}
+
+/* Creates the internal functions for spawn helper and parent.  */
+
+void
+c_install_body_with_frame_cleanup (tree fndecl, tree body)
+{

Please document function arguments.

+  tree list;
+  tree frame = make_cilk_frame (fndecl);
+  tree dtor = build_cilk_function_exit (frame, false, false);
+  add_local_decl (cfun, frame);
+
+  DECL_SAVED_TREE (fndecl) = (list = alloc_stmt_list ());
+  append_to_statement_list_force (build_stmt (EXPR_LOCATION (body),
+                                             TRY_FINALLY_EXPR, body, dtor),
+                                 &list);
+}
+
+/* Add a new variable, VAR to a variable list in WD->DECL_MAP.  */
+
+static void
+add_variable (struct wrapper_data *wd, tree var, enum add_variable_type how)

Please document HOW.

+{
+  void **valp;
+
+  valp = pointer_map_contains (wd->decl_map, (void *) var);
+  if (valp)
+    {
+      tree val = (tree) *valp;
+      /* If the variable is local, do nothing.  */
+      if (val == error_mark_node)
+       return;
+      /* If the variable was entered with itself as value,
+        meaning it belongs to an outer scope, do not alter
+        the value.  */
+      if (val == var)
+       return;
+      /* A statement expression may cause a variable to be
+        bound twice, once in BIND_EXPR and again in a
+        DECL_EXPR.  That case caused a return in the
+        test above.  Any other duplicate definition is
+        an error.  */
+      gcc_assert (how != ADD_BIND);
+      if (how != ADD_WRITE)
+       return;
+      /* This variable might have been entered as read but is now written.  */
+      *valp = (void *) var;
+      wd->nested = true;
+      return;
+    }
+  else
+    {
+      tree val = NULL_TREE;
+
+      /* Nested function rewriting silently discards hard register
+        assignments for function scope variables, and they wouldn't
+        work anyway.  Warn here.  This misses one case: if the
+        register variable is used as the loop bound or increment it
+        has already been added to the map.  */
+      if ((how != ADD_BIND) && (TREE_CODE (var) == VAR_DECL)
+         && !DECL_EXTERNAL (var) && DECL_HARD_REGISTER (var))
+       warning (0, "register assignment ignored for %qD used in Cilk block",
+                var);
+
+      switch (how)
+       {
+         /* ADD_BIND means always make a fresh new variable.  */
+       case ADD_BIND:
+         val = error_mark_node;
+         break;
+         /* ADD_READ means
+            1. For cilk_for, refer to the outer scope definition as-is
+            2. For a spawned block, take a scalar in an argument
+            and otherwise refer to the outer scope definition as-is.
+            3. For a spawned call, take a scalar in an argument.  */
+       case ADD_READ:
+         switch (wd->type)
+           {
+           case CILK_BLOCK_FOR:
+             val = var;
+             break;
+           case CILK_BLOCK_SPAWN:
+             if (TREE_ADDRESSABLE (var))
+               {
+                 val = var;
+                 wd->nested = true;
+                 break;
+               }
+             val = integer_zero_node;
+             break;
+           }
+         break;
+       case ADD_WRITE:
+         switch (wd->type)
+           {
+           case CILK_BLOCK_FOR:
+             val = var;
+             wd->nested = true;
+             break;
+           case CILK_BLOCK_SPAWN:
+             if (TREE_ADDRESSABLE (var))
+               val = integer_one_node;
+             else
+               {
+                 val = var;
+                 wd->nested = true;
+               }
+             break;
+           }
+       }
+      *pointer_map_insert (wd->decl_map, (void *)var) = val;
+    }
+}
+
+/* Find the variables referenced in an expression T.  This does not avoid
+   duplicates because a variable may be read in one context and written in
+   another.  HOW describes the context in which the reference is seen.  If
+   NESTED is true a nested function is being generated and variables in the
+   original context should not be remapped.  */
+
+static void
+extract_free_variables (tree t, struct wrapper_data *wd,
+                       enum add_variable_type how)
+{
+#define SUBTREE(EXP)  extract_free_variables (EXP, wd, ADD_READ)
+#define MODIFIED(EXP) extract_free_variables (EXP, wd, ADD_WRITE)
+#define INITIALIZED(EXP) extract_free_variables (EXP, wd, ADD_BIND)
+
+  if (t == NULL_TREE)
+    return;
+
+  enum tree_code code = TREE_CODE (t);
+  bool is_expr = IS_EXPR_CODE_CLASS (TREE_CODE_CLASS (code));
+  location_t loc =  EXPR_LOCATION (t);
+
+  if (is_expr)
+    SUBTREE (TREE_TYPE (t));
+
+  switch (code)
+    {
+    case ERROR_MARK:
+    case IDENTIFIER_NODE:
+    case INTEGER_CST:
+    case REAL_CST:
+    case FIXED_CST:
+    case STRING_CST:
+    case BLOCK:
+    case PLACEHOLDER_EXPR:
+    case FIELD_DECL:
+    case VOID_TYPE:
+    case REAL_TYPE:
+      /* These do not contain variable references.  */
+      return;
+
+    case SSA_NAME:
+      /* Currently we don't see SSA_NAME.  */
+      extract_free_variables (SSA_NAME_VAR (t), wd, how);
+      return;
+
+    case LABEL_DECL:
+      /* This might be a reference to a label outside the Cilk block,
+        which is an error, or a reference to a label in the Cilk block
+        that we haven't seen yet.  We can't tell.  Ignore it.  An
+        invalid use will cause an error later in copy_decl_for_cilk.  */
+      return;
+
+    case RESULT_DECL:
+      if (wd->type != CILK_BLOCK_SPAWN)
+       TREE_ADDRESSABLE (t) = 1;
+    case VAR_DECL:
+    case PARM_DECL:
+      if (!TREE_STATIC (t) && !DECL_EXTERNAL (t))
+       add_variable (wd, t, how);
+      return;
+
+    case NON_LVALUE_EXPR:
+    case CONVERT_EXPR:
+    case NOP_EXPR:
+      SUBTREE (TREE_OPERAND (t, 0));
+      return;
+
+    case INIT_EXPR:
+      INITIALIZED (TREE_OPERAND (t, 0));
+      SUBTREE (TREE_OPERAND (t, 1));
+      return;
+
+    case MODIFY_EXPR:
+    case PREDECREMENT_EXPR:
+    case PREINCREMENT_EXPR:
+    case POSTDECREMENT_EXPR:
+    case POSTINCREMENT_EXPR:
+      /* These write their result.  */
+      MODIFIED (TREE_OPERAND (t, 0));
+      SUBTREE (TREE_OPERAND (t, 1));
+      return;
+
+    case ADDR_EXPR:
+      /* This might modify its argument, and the value needs to be
+        passed by reference in any case to preserve identity and
+        type if is a promoting type.  In the case of a nested loop
+        just notice that we touch the variable.  It will already
+        be addressable, and marking it modified will cause a spurious
+        warning about writing the control variable.  */
+      if (wd->type != CILK_BLOCK_SPAWN)
+       SUBTREE (TREE_OPERAND (t, 0));
+      else
+       MODIFIED (TREE_OPERAND (t, 0));
+      return;
+
+    case ARRAY_REF:
+      /* Treating ARRAY_REF and BIT_FIELD_REF identically may
+        mark the array as written but the end result is correct
+        because the array is passed by pointer anyway.  */
+    case BIT_FIELD_REF:
+      /* Propagate the access type to the object part of which
+        is being accessed here.  As for ADDR_EXPR, don't do this
+        in a nested loop, unless the access is to a fixed index.  */
+      if (wd->type != CILK_BLOCK_FOR || TREE_CONSTANT (TREE_OPERAND (t, 1)))
+       extract_free_variables (TREE_OPERAND (t, 0), wd, how);
+      else
+       SUBTREE (TREE_OPERAND (t, 0));
+      SUBTREE (TREE_OPERAND (t, 1));
+      SUBTREE (TREE_OPERAND (t, 2));
+      return;
+
+    case TREE_LIST:
+      SUBTREE (TREE_PURPOSE (t));
+      SUBTREE (TREE_VALUE (t));
+      SUBTREE (TREE_CHAIN (t));
+      return;
+
+    case TREE_VEC:
+      {
+       int len = TREE_VEC_LENGTH (t);
+       int i;
+       for (i = 0; i < len; i++)
+         SUBTREE (TREE_VEC_ELT (t, i));
+       return;
+      }
+
+    case VECTOR_CST:
+      {
+       unsigned ii = 0;
+       for (ii = 0; ii < VECTOR_CST_NELTS (t); ii++)
+         SUBTREE (VECTOR_CST_ELT (t, ii));
+       break;
+      }
+
+    case COMPLEX_CST:
+      SUBTREE (TREE_REALPART (t));
+      SUBTREE (TREE_IMAGPART (t));
+      return;
+
+    case BIND_EXPR:
+      {
+       tree decl;
+       for (decl = BIND_EXPR_VARS (t); decl; decl = TREE_CHAIN (decl))
+         {
+           add_variable (wd, decl, ADD_BIND);
+           /* A self-referential initialization is no problem because
+              we already entered the variable into the map as local.  */
+           SUBTREE (DECL_INITIAL (decl));
+           SUBTREE (DECL_SIZE (decl));
+           SUBTREE (DECL_SIZE_UNIT (decl));
+         }
+       SUBTREE (BIND_EXPR_BODY (t));
+       return;
+      }
+
+    case STATEMENT_LIST:
+      {
+       tree_stmt_iterator i;
+       for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))
+         SUBTREE (*tsi_stmt_ptr (i));
+       return;
+      }
+
+    case OMP_PARALLEL:
+    case OMP_TASK:
+    case OMP_FOR:
+    case OMP_SINGLE:
+    case OMP_SECTION:
+    case OMP_SECTIONS:
+    case OMP_MASTER:
+    case OMP_ORDERED:
+    case OMP_CRITICAL:
+    case OMP_ATOMIC:
+    case OMP_CLAUSE:
+      error_at (loc, "OMP construct used within Cilk construct");

No corresponding test.

+      break;
+
+    case TARGET_EXPR:
+      {
+       INITIALIZED (TREE_OPERAND (t, 0));
+       SUBTREE (TREE_OPERAND (t, 1));
+       SUBTREE (TREE_OPERAND (t, 2));
+       if (TREE_OPERAND (t, 3) != TREE_OPERAND (t, 1))
+         SUBTREE (TREE_OPERAND (t, 3));
+       return;
+      }
+
+    case RETURN_EXPR:
+      if (TREE_NO_WARNING (t))
+       {
+         gcc_assert (errorcount);
+         return;
+       }
+      error_at (loc, "spawn of return statement not allowed");
+      return;

No testcase.

+
+    case DECL_EXPR:
+      if (TREE_CODE (DECL_EXPR_DECL (t)) != TYPE_DECL)
+       INITIALIZED (DECL_EXPR_DECL (t));
+      return;
+
+    case INTEGER_TYPE:
+    case ENUMERAL_TYPE:
+    case BOOLEAN_TYPE:
+      SUBTREE (TYPE_MIN_VALUE (t));
+      SUBTREE (TYPE_MAX_VALUE (t));
+      return;
+
+    case POINTER_TYPE:
+      SUBTREE (TREE_TYPE (t));
+      break;
+
+    case ARRAY_TYPE:
+      SUBTREE (TREE_TYPE (t));
+      SUBTREE (TYPE_DOMAIN (t));
+      return;
+
+    case RECORD_TYPE:
+      SUBTREE (TYPE_FIELDS (t));
+      return;
+
+    case METHOD_TYPE:
+      SUBTREE (TYPE_ARG_TYPES (t));
+      SUBTREE (TYPE_METHOD_BASETYPE (t));
+      return;
+
+    case AGGR_INIT_EXPR:
+    case CALL_EXPR:
+      {
+       int len = 0;
+       int ii = 0;
+       if (TREE_CODE (TREE_OPERAND (t, 0)) == INTEGER_CST)
+         {
+           len = TREE_INT_CST_LOW (TREE_OPERAND (t, 0));
+
+           for (ii = 0; ii < len; ii++)
+             SUBTREE (TREE_OPERAND (t, ii));
+           SUBTREE (TREE_TYPE (t));
+         }
+       break;
+      }
+
+    default:
+      if (is_expr)
+       {
+         int i, len;
+
+         /* Walk over all the sub-trees of this operand.  */
+         len = TREE_CODE_LENGTH (code);
+
+         /* Go through the subtrees.  We need to do this in forward order so
+            that the scope of a FOR_EXPR is handled properly.  */
+         for (i = 0; i < len; ++i)
+           SUBTREE (TREE_OPERAND (t, i));
+       }
+      return;

This return is ambiguous.

+    }
+}
+
+
+/* Add appropriate frames needed for a Cilk spawned function call, FNDECL.
+   Returns the __cilkrts_stack_frame * variable.  */
+
+tree
+insert_cilk_frame (tree fndecl)
+{
+  tree addr, body, enter , out, orig_body;

extra space after "enter "

+  location_t loc = EXPR_LOCATION (fndecl);
+
+  if (!cfun || cfun->decl != fndecl)
+    push_cfun (DECL_STRUCT_FUNCTION (fndecl));
+
+  tree decl = cfun->cilk_frame_decl;
+  if (!decl)
+    {
+      tree *saved_tree = &DECL_SAVED_TREE (fndecl);
+      decl = make_cilk_frame (fndecl);
+      add_local_decl (cfun, decl);
+
+      addr = build1 (ADDR_EXPR, cilk_frame_ptr_type_decl, decl);
+      enter = build_call_expr (cilk_enter_fndecl, 1, addr);
+      out = build_cilk_function_exit (cfun->cilk_frame_decl, false, true);
+
+      /* The new body will be:
+        __cilkrts_enter_frame_1 (&sf);
+        try {
+           orig_body;
+        }
+        finally {
+            __cilkrts_pop_frame (&sf);
+            __cilkrts_leave_frame (&sf);
+         }  */
+
+      body = alloc_stmt_list ();
+      orig_body = *saved_tree;
+
+      if (TREE_CODE (orig_body) == BIND_EXPR)
+       orig_body = BIND_EXPR_BODY (orig_body);
+
+      append_to_statement_list (enter, &body);
+      append_to_statement_list (build_stmt (loc, TRY_FINALLY_EXPR, orig_body,
+                                           out), &body);
+      if (TREE_CODE (*saved_tree) == BIND_EXPR)
+       BIND_EXPR_BODY (*saved_tree) = body;
+      else
+       *saved_tree = body;
+    }
+  return decl;
+}

By the way, it would be nice if you could pay closer attention to minutae and other things that are specified in the coding standards: two spaces after periods, comments on their own lines, remember to document arguments to functions (unless callbacks and obvious things like that), misspellings, extra capitalization, extra whitespace, etc etc. It takes a long time to go through big patchsets only to find many instances of things that have been pointed out before.

Aldy

Reply via email to