@@ -960,6 +960,7 @@ SCEV_H = tree-scalar-evolution.h $(GGC_H) tree-chrec.h
$(PARAMS_H)
OMEGA_H = omega.h $(PARAMS_H)
TREE_DATA_REF_H = tree-data-ref.h $(OMEGA_H) graphds.h $(SCEV_H)
TREE_INLINE_H = tree-inline.h
+CILK_H = cilk.h
REAL_H = real.h $(MACHMODE_H)
IRA_INT_H = ira.h ira-int.h $(CFGLOOP_H) alloc-pool.h
Doesn't cilk.h depend on tree.h? So shouldn't that be:
CILK_H = cilk.h $(TREE_H)
Which would mean that whoever includes cilk.h doesn't need to include
tree.h. Although... what I would prefer is not to include tree.h from
cilk.h at all, and have the caller/includer of cilk.h include tree.h first.
+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) $(C_COMMON_H) langhooks.h
TREE_H is duplicated.
Also, you are using DIAGNOSTIC_CORE_H whereas c-family/cilk.c is using
+#include "diagnostic.h"
+/* Cilk Keywords builtins. */
+#include "cilk-builtins.def"
keywords should be lowercase
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index dc430c3..ab77fa4 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -148,6 +148,9 @@ enum rid
/* C++11 */
RID_CONSTEXPR, RID_DECLTYPE, RID_NOEXCEPT, RID_NULLPTR, RID_STATIC_ASSERT,
+ /* Cilk Plus Keywords. */
+ RID_CILK_SPAWN, RID_CILK_SYNC,
+
Same here.
+/* In cilk.c. */
+extern tree insert_cilk_frame (tree);
+extern void cilk_init_builtins (void);
+extern int gimplify_cilk_spawn (tree *, gimple_seq *, gimple_seq *);
+extern int gimplify_cilk_sync (tree *, gimple_seq *, gimple_seq *);
+extern void c_cilk_install_body_w_frame_cleanup (tree, tree);
+extern bool cilk_valid_spawn (tree *);
+extern bool cilkplus_set_spawn_marker (location_t, tree);
You should be consistent with the prefix throughout the entire patch.
For instance, now you have cilk_init_builtins() but
cilkplus_set_spawn_marker(). Do whatever you like, but be consistent.
Also, insert_cilk_frame-- the prefix should be well...as a prefix should
be...at the beginning. However, gimplify_cilk_spawn/sync is ok since
that is in keeping with the rest of the gimplify.c style.
+ /* IF the function has _Cilk_spawn in front of a function call inside it
+ i.e. it is a spawning function, then add the appropriate Cilk plus
+ functions inside. */
Lowercase the F in IF.
+ c_parser_skip_until_found (parser, CPP_SEMICOLON, "expected %<;%>");
+ if (!flag_enable_cilkplus)
+ error_at (loc, "-fcilkplus must be enabled to use _Cilk_sync");
The error message should use %<_Cilk_sync>
+ case RID_CILK_SPAWN:
+ c_parser_consume_token (parser);
+ if (!flag_enable_cilkplus)
+ {
+ error_at (loc, "-fcilkplus must be enabled to use _Cilk_spawn");
+ expr = c_parser_postfix_expression (parser);
+ expr.value = error_mark_node;
Similarly with _Cilk_spawn.
+ if (c_parser_peek_token (parser)->keyword == RID_CILK_SPAWN)
+ {
+ error_at (input_location, "consecutive _Cilk_spawn keywords "
+ "are not permitted");
Similarly here-- for that matter, check the rest of the patch for any
keywords in error messages, they should have %<blah>.
Also, avoid input_location when possible. Surely, you can infer the
actual location from the parser.
+ if (flag_enable_cilkplus && retval && TREE_CODE (retval) == CILK_SPAWN_STMT)
+ {
+ error_at (loc, "use of _Cilk_spawn in return statement is not allowed");
+ return error_mark_node;
s/in return/in a return/. Also, use %<_Cilk_spawn>.
+/* Returns a tree of type CILK_SYNC_STMT if Cilk Plus is enabled. Otherwise
+ return error_mark_node. */
+
+tree
+c_build_cilk_sync (void)
+{
+ tree sync = build0 (CILK_SYNC_STMT, void_type_node);
+ TREE_SIDE_EFFECTS (sync) = 1;
+ return sync;
+}
Code seems correct and comment wrong. Adjust accordingly.
diff --git a/gcc/cilk.h b/gcc/cilk.h
new file mode 100644
index 0000000..038316a
--- /dev/null
+++ b/gcc/cilk.h
@@ -0,0 +1,94 @@
+/* This file is part of the Intel(R) Cilk(TM) Plus support
+ This file contains Cilk Support files.
+ Copyright (C) 2013 Free Software Foundation, Inc.
Only one space after 2013. Similarly in other files.
+
+#ifndef GCC_CILK_H
+#define GCC_CILK_H
+
+#include "tree.h"
As mentioned earlier, cilk.h should not include tree.h, the caller should.
+/* Frame status bits known to compiler. */
+#define CILK_FRAME_STOLEN 0x01
Unused?
+this keyword with a set of functions that are stored in the Cilk Runtime.
Lowercase "Runtime".
+@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 @code{_Cilk_sync}:
+@code{_Cilk_sync} is parsed like a keyword. 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 the Cilk Runtime.
+One of the internal functions inserted during gimplification,
+@code{__cilkrts_pop_frame} must be expanded by the compiler and is
+done by @code{builtin_expand_cilk_pop_frame} in @file{cilk-common.c}.
Here and elsewhere in the documentation... It's up to you, but I would
prefer to remove reference to the actual file the functions are defined
in, because functions could be rearranged later.
+ case CILK_SYNC_STMT:
+ {
+ if (flag_enable_cilkplus)
+ {
+ if (!cfun->cilk_frame_decl)
+ {
+ error_at (input_location, "expected _Cilk_spawn before "
+ "_Cilk_sync");
+ ret = GS_ERROR;
+ }
+ else
+ ret = (enum gimplify_status)
+ lang_hooks.cilkplus.gimplify_cilk_sync (expr_p, pre_p,
+ post_p);
+ break;
+ }
+ else
+ /* _Cilk_sync without Cilk Plus enabling should be caught by
+ the parser. */
+ gcc_unreachable ();
The check for !flag_enable_cilkplus is not necessary, as well as the
gcc_unreachable. You shouldn't be checking for flag_enable_cilkplus in
the gimplifier; an earlier pass (parser?) should've caught this. See
how TRANSACTION_EXPR is handled.
Also, I would use the location of the CILK_SYNC_STMT instead of
input_location.
diff --git a/gcc/ipa-inline-analysis.c b/gcc/ipa-inline-analysis.c
index 9a36292..7c63cc8 100644
--- a/gcc/ipa-inline-analysis.c
+++ b/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_cilk_spawn)
+ /* We can't inline if the function is spawing a function. */
+ e->inline_failed = CIF_FUNCTION_NOT_INLINABLE;
Didn't rth mention that instead of looking through cfun (which may or
may not be there), you could check the callee/caller edge?
+void
+lhd_install_body_with_frame_cleanup (tree, tree)
+{
+ return;
+}
As mentioned before, avoid empty returns.
+ /* Returns true if the call expr passed is a spawned function call. */
Here and elsewhere, you probably want s/call expr/CALL_EXPR/g
diff --git a/gcc/tree-inline.h b/gcc/tree-inline.h
index b65dee9..32bedac 100644
--- a/gcc/tree-inline.h
+++ b/gcc/tree-inline.h
@@ -123,6 +123,10 @@ typedef struct copy_body_data
the originals have been mapped to a value rather than to a
variable. */
struct pointer_map_t *debug_map;
+
+ /* Cilk keywords currently need to replace some variables that
+ ordinary nested functions do not. */
+ bool remap_var_for_cilk;
} copy_body_data;
Where is this field used?
diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c
index 7745f73..f449c68 100644
--- a/gcc/tree-pretty-print.c
+++ b/gcc/tree-pretty-print.c
@@ -2403,6 +2403,14 @@ dump_generic_node (pretty_printer *buffer, tree node,
int spc, int flags,
dump_block_node (buffer, node, spc, flags);
break;
+ case CILK_SPAWN_STMT:
+ pp_string (buffer, "_Cilk_spawn ");
+ dump_generic_node (buffer, TREE_OPERAND (node, 0), spc, flags, false);
+ break;
+
+ case CILK_SYNC_STMT:
+ pp_string (buffer, "_Cilk_sync;");
+ break;
default:
NIY;
In keeping with the present code in this switch statement, empty line
before the default.
+/* Cilk spawn expression
+ Operand 0 is the CALL_EXPR. */
+DEFTREECODE (CILK_SPAWN_STMT, "cilk_spawn_stmt", tcc_statement, 1)
+
+/* Cilk Sync Statement: Does not have any operands. */
+DEFTREECODE (CILK_SYNC_STMT, "cilk_sync_stmt", tcc_expression, 0)
Be consistent. "Cilk spawn" but then "Cilk Sync"? Also "expression" in
one, but "statement" in the next? Also, Statement should be lowercase,
as well as Sync.
+/* Cilk Keywords accessors. */
+#define CILK_SPAWN_FN(NODE) TREE_OPERAND (CILK_SPAWN_STMT_CHECK (NODE), 0)
+
Lowercase "Keywords".
+ if (!current_function_decl)
+ {
extra whitespace after {
+ else if (TREE_CODE (fcall) != CALL_EXPR)
+ {
extra whitespace after {
diff --git a/gcc/testsuite/c-c++-common/cilk-plus/CK/spawnee_inline.c
b/gcc/testsuite/c-c++-common/cilk-plus/CK/spawnee_inline.c
new file mode 100644
index 0000000..daf932e
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/cilk-plus/CK/spawnee_inline.c
@@ -0,0 +1,81 @@
+/* { dg-do compile } */
+/* { dg-do run { target { i?86-*-* x86_64-*-* arm*-*-* } } } */
+/* { dg-options "-fcilkplus -w" } */
+/* { dg-options "-lcilkrts" { target { i?86-*-* x86_64-*-* arm*-*-* } } } */
+
+#include <stdio.h>
+#include <stdlib.h>
+#define DEFAULT_VALUE "30"
+
+int fib (char *n_char)
+{
+ int n;
+ char n_char_minus_one[20], n_char_minus_two[20];
+ if (n_char)
+ n = atoi (n_char);
+ else
+ n = atoi(DEFAULT_VALUE);
+
+ if (n < 2)
+ return n;
+ else
+ {
Extra whitespace after last {
diff --git a/gcc/testsuite/c-c++-common/cilk-plus/CK/spawning_arg.c
b/gcc/testsuite/c-c++-common/cilk-plus/CK/spawning_arg.c
new file mode 100644
index 0000000..465d1da
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/cilk-plus/CK/spawning_arg.c
@@ -0,0 +1,38 @@
+/* { dg-do compile } */
+/* { dg-do run { target { i?86-*-* x86_64-*-* arm*-*-* } } } */
+/* { dg-options "-fcilkplus" } */
+/* { dg-options "-lcilkrts" { target { i?86-*-* x86_64-*-* arm*-*-* } } } */
+
+void f0(volatile int *steal_flag)
+{
same
+/* This function does whatever is necessary to make the compiler emit a newly
Extra space at end
+/* 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. */
Please add empty lines between paragraphs.
+static bool
+cilk_spawnable_constructor (tree exp)
+{
+ if (TREE_CODE (exp) != ADDR_EXPR)
+ return false;
Is this function ever called with a non ADDR_EXPR?
+/* 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
two many spaces after "and"
Also, do not document what you are doing in the code by repeating it.
For instance, instead of "*WALK_SUBTREES is set to 0", either omit this,
or say what is being accomplished by setting WALK_SUBTREES to 0. I
would just omit it.
+ /* Remove the CALL_EXPR from CILK_SPAWN_STMT wrapper and return true. */
Again, don't document the obvious. No need to say "and return true".
That much is obvious from the code.
+ /* Happens with C++ TARGET_EXPR. */
+ if (exp == NULL_TREE)
+ return false;
Extra space after false;
+ 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");
This warning seems very ambiguous. Do you think perhaps you could issue
a more meaningful error in cilk_spawnable_constructor()?
+/* This function will return a FNDECL using information from *WD. */
+
+static tree
+build_cilk_helper_decl (struct wrapper_data *wd)
I think a more meaningful comment is "This function will build and
return a FUNCTION_DECL using..."
+ case LABEL_DECL:
+ error_at (EXPR_LOCATION (decl), "invalid use of label %q+D in spawn",
+ decl);
+ return error_mark_node;
Be consistent. If you're using _Cilk_spawn then use that. And you
probably want %<_Cilk_spawn> or whatever.
+ /* Copy FROM the function containing the spawn... */
+ id.src_fn = outer_fn;
+
+ /* ...TO the wrapper. */
Lower case FROM and TO.
