Re: [PATCH] Repair __builtin_setjmp/__builtin_longjmp

[Steven Bosscher <stevenb dot gcc at gmail dot com>]

On Thursday 21 September 2006 22:45, Eric Botcazou wrote:
>	* builtins.c (expand_builtin_setjmp): Delete.

This makes the gcc world a better place :-)

> 	* tree-optimize.c (has_abnormal_outgoing_edge_p): New predicate.
This really belongs in cfganal.c or some other common-to-rtl-and-trees
kind of place.


>	(execute_fixup_cfg): If there are non-local labels in the function,
>	scan the basic blocks and split them at calls that can go to non-local
>	labels or add missing abnormal call edges.  Write down the CFG in the
>	dump file.
It'd be nice to have such a comment in the function itself, too.


You have this in expand_builtin:
> +	  /* This is copied from the handling of non-local gotos.  */
> +	  expand_builtin_setjmp_setup (buf_addr, label_r);
> +	  nonlocal_goto_handler_labels
> +	    = gen_rtx_EXPR_LIST (VOIDmode, label_r,
> +				 nonlocal_goto_handler_labels);
> +	  /* ??? Do not let expand_label treat us as such.  */
>   FORCED_LABEL (label) = 0;
Could you elaborate a bit about the "why not" in the comment?  For
non-local gotos, afaik, the reachable labels are always forced, so
why not in this case?


For a program with enough (say, N) builtin setjmp/longjmps, this code:
> +    case BUILT_IN_SETJMP_DISPATCHER:
> +       /* __builtin_setjmp_dispatcher is passed the dispatcher label.  */
> +      if (validate_arglist (arglist, POINTER_TYPE, VOID_TYPE))
> +	{
> +	  tree label = TREE_OPERAND (TREE_VALUE (arglist), 0);
> +	  rtx label_r = label_rtx (label);
> +
> +	  /* Remove the dispatcher label from the list of non-local labels
> +	     since the receiver labels have been added to it above.  */
> +	  remove_node_from_expr_list (label_r, &nonlocal_goto_handler_labels);
> +	  return const0_rtx;
> +	}
> +      break;
will show quadratic behavior (adding/removing labels to the list
of nonlocal_goto_handler_labels, which means you're traversing N
times a list of length N/2 ...).  There has to be a better way...?


> @@ -4059,7 +4058,7 @@ tree_redirect_edge_and_branch (edge e, b
>    edge ret;
>    tree label, stmt;
>  
> -  if (e->flags & (EDGE_ABNORMAL_CALL | EDGE_EH))
> +  if (e->flags & EDGE_ABNORMAL)
>      return NULL;
>  
>    if (e->src != ENTRY_BLOCK_PTR

Perhaps better to have this here:
> +  if (e->flags & EDGE_COMPLEX)

HTH,

Gr.
Steven