On 10/16/13 15:49, Iyer, Balaji V wrote:
In ira.c:
+ /* We need a frame pointer for all Cilk Plus functions that use
+ Cilk keywords. */
+ || (flag_enable_cilkplus && cfun->is_cilk_function)
Can you explain to me a bit more why you need a frame pointer? I'm trying to
determine if it's best to leave this as-is or have this code detect a property
in the
generated code for the function. From a modularity standpoint it seems pretty
gross that we have to peek at this within IRA.
Cilk Runtime functions changes the stack pointer. So, frame pointer is
necessary.
Nevermind -- that seems to be the location where this is detected now.
So this is fine.
In a couple places I saw this comment:
+ /* Cilk keywords currently need to replace some variables that
+ ordinary nested functions do not. */ bool remap_var_for_cilk;
I didn't see anywhere that explained exactly why some variables that do not
ordinarily need replacing need replacing when cilk is enabled. If it's in the
patch
somewhere, just point me to it. If not, add documentation about why these
variables need remapping for cilk.
It is used in the cilk_outline function.
Thanks. Presumably the comment "We don't want the private variables
anymore" is the relevant code/comment?
Does anything actually ensure we don't have multiple syncs?
Well, _Cilk_sync expands to something like this:
If (!sync_occurred)
__cilkrts_sync()
So, having multiple Cilk syncs doesn't harm, just that the then case of the
if-statement will not be taken.
OK. Thanks.
What's the thinking behind parsing calls to cilk_spawn as a normal call if
there's
an error? Referring to this code in gimplify.c:
+ case CILK_SPAWN_STMT:
+ gcc_assert
+ (fn_contains_cilk_spawn_p (cfun)
+ && lang_hooks.cilkplus.cilk_detect_spawn_and_unwrap (expr_p));
+ if (!seen_error ())
+ {
+ ret = (enum gimplify_status)
+ lang_hooks.cilkplus.gimplify_cilk_spawn (expr_p, pre_p,
+ post_p);
+ break;
+ }
+ /* If errors are seen, then just process it as a CALL_EXPR.
+ */
+
Well, if there is an error the compiler is not going to produce an executable.
So, I just let the compiler go far as it can and catch all the other errors. If
the error is cilk related, we have already called them out on it. Adding
_Cilk_spawn specific routines would add additional complication.
I guess that's a reasonable fallback position in case of an error.
Meta-question, when we're not in cilk mode, should we be consuming the cilk
tokens? I'm not familiar at all with our parser, so I'm not sure if we can
handle
this gracefully. Though I guess parsing hte token and warning/error if not in
Cilk
mode is probably the best course of action.
In the compiler, I couldn't make conditional tokens. When the parser hits a
_Cilk_spawn or _Cilk_sync token, it will check if Cilk Plus is enabled or will
complain. Now that I think about it in detail, I suppose it will also block if
anyone wants to have a variable name called _Cilk_spawn or _Cilk_sync and not
using -fcilkplus. But, they start with '_', and so I guess it is not a normal
case.
Figured it was ugly at best to avoid consuming the cilk tokens when not
in cilk mode.
Can you take a look at calls.c::special_function_p and determine if we need to
do something special for spawn here?
I will look into it and let you know.
Any word on this?
jeff
