--- 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?
Yes, I am expanding some variables of type "ptr->value" and those are emitted
as INDIRECT_REF.
The fact that you are getting an INDIRECT_REF this late in the game is
suspect.
Are you building with ENABLE_CHECKING, because it seems this should have
been caught. See the places in tree-cfg.c with this:
case INDIRECT_REF:
error ("INDIRECT_REF in gimple IL");
return t;
+ /* 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.
I also have a limited knowledge of LTO. This seem to be the most
straightforward way of doing it (atleast for me).
See how other bits in `struct function' are streamed in/out in LTO, for
example in output_struct_function_base()
bp_pack_value (&bp, fn->calls_alloca, 1);
bp_pack_value (&bp, fn->calls_setjmp, 1);
bp_pack_value (&bp, fn->va_list_fpr_size, 8);
bp_pack_value (&bp, fn->va_list_gpr_size, 8);
and the corresponding in input_struct_function_base():
fn->calls_alloca = bp_unpack_value (&bp, 1);
fn->calls_setjmp = bp_unpack_value (&bp, 1);
fn->va_list_fpr_size = bp_unpack_value (&bp, 8);
fn->va_list_gpr_size = bp_unpack_value (&bp, 8);
Also, you will need a testcase to make sure later changes to the
compiler do not break LTO wrt Cilk features you have added.
+ /* 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/
It should be keywords, because you need frame-pointer for "_Cilk_spawn and _Cilk_sync"
and "_Cilk_for"
I meant that you should lowercase the "K".
+ /* This variable will tell whether we are on a spawn helper or not */
+ unsigned int is_cilk_helper_function : 1;
Where is this used?
Well, it is not used now but later on when I add Tools support it will be. I
will remove it for now.
Yes, please.
--- 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?
When I am first setting this, I don't think cgraph is available.
See Richard's comment with regards to struct function and its
availability via the callee edge. Also see his comment regarding the
inappropriate error message.
@@ -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 ;
What's this for?
Added a new enum field called BUILT_IN_CILK so we need 3 bits instead of 2,
since there are 5 fields instead of 4.
Hmm, yeah. I see you added another field here:
diff --git a/gcc/tree.h b/gcc/tree.h
index 0058a4b..952362f 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -262,6 +262,7 @@ enum built_in_class
NOT_BUILT_IN = 0,
BUILT_IN_FRONTEND,
BUILT_IN_MD,
+ BUILT_IN_CILK,
BUILT_IN_NORMAL
};
If you look at the comment above enum built_in_class, you will see that
these classes specify which part of the compiler created the built-in
(the frontend, the backend (MD), or a normal builtin). I don't see how
Cilk should be treated specially. And even so, I don't see how you use
this BUILT_IN_CILK class anywhere.
+ 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;
That would be necessary for C++, but it returns false for C. So, should I take
out this hook for now? Would prefer to keep it in
You can keep the hook, but do put a comment specifying that it's a place
holder for C++. And also, please remove the second return.
Are these two hooks ever set to anything but hook_bool_tree_false? If
so, why the need for them?
Used in C++ but not in C.
Leave them. Similar comment please.
+ 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?
That is how the field is described in Cilk Runtime source (please see:
http://gcc.gnu.org/svn/gcc/branches/cilkplus/libcilkrts/include/internal/abi.h)
Ok, I'm not a language expert, I presume this is specifying volatile on
both indirections and is proper form.
+/* 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.
The reason why I did this is that, there is a generic empty hook for this in
langhooks.c that returns int. So, to reduce the number of code addition, I just
made it return int. Also in future, if i want to add more things, having a
return value I thought would be helpful.
Use void. Set GS_ALL_DONE in the caller. You can add the return value
when you use it in the future.
Aldy
