Add polymorphic call context propagation to ipa-prop

Martin Jambor mjambor@suse.cz
Tue Oct 21 13:49:00 GMT 2014


Hi,


On Thu, Oct 02, 2014 at 09:00:12AM +0200, Jan Hubicka wrote:
> Hi,
> this patch makes ipa-prop to use ipa-polymorphic-call-context infrastructure
> for forward propagation (in a very minimal and simple way). 
> 
> At the moment only static type info is propagated and it is used only
> speculatively because I will need to update dynamic type change code to deal
> with more general setting than in the old binfo propagation code. I want to do
> it incrementally.  The basic problem is that the old binfo code wasmostly built
> around idea that all bases are primary bases and one class may contain other
> only as a base (not as field).
> 
> Martin, I get out of range ICEs in controlled uses code and thus
> I added an extra check, see FIXME bellow. Could you, please, help
> me to fix that correctly?

Is there a simple testcase?

And sorry for not reviewing this in time but I've only recently
noticed that...

> 
> THe patch also does not add necessary propagation into ipa-cp and thus
> devirtualizatoin happens only during inlining and is not appropriately
> hinted.
> 
> Bootstrapped/regtested x86_64-linux, will commit it shortly.
> 
> Honza
> 
> 	* ipa-prop.h (ipa_get_controlled_uses): Add hack to avoid ICE
> 	when speculation is added.
> 	(ipa_edge_args): Add polymorphic_call_contexts.
> 	(ipa_get_ith_polymorhic_call_context): New accesor.
> 	(ipa_make_edge_direct_to_target): Add SPECULATIVE parameter.
> 	(ipa_print_node_jump_functions_for_edge): Print contexts.
> 	(ipa_compute_jump_functions_for_edge): Compute contexts.
> 	(update_jump_functions_after_inlining): Update contexts.
> 	(ipa_make_edge_direct_to_target): Add SPECULATIVE argument;
> 	update dumping; add speculative edge creation.
> 	(try_make_edge_direct_virtual_call): Add CTX_PTR parameter; handle
> 	context updating.
> 	(update_indirect_edges_after_inlining): Pass down context.
> 	(ipa_edge_duplication_hook): Duplicate contexts.
> 	(ipa_write_node_info): Stream out contexts.
> 	(ipa_read_node_info): Stream in contexts.
> 	* ipa-devirt.c (type_all_derivations_known_p): Avoid ICE on non-ODR
> 	types.
> 	(try_speculative_devirtualization): New function.
> 	* ipa-utils.h (try_speculative_devirtualization): Declare.
> Index: ipa-prop.h
> ===================================================================
> --- ipa-prop.h	(revision 215792)
> +++ ipa-prop.h	(working copy)
> @@ -432,7 +432,10 @@ ipa_set_param_used (struct ipa_node_para
>  static inline int
>  ipa_get_controlled_uses (struct ipa_node_params *info, int i)
>  {
> -  return info->descriptors[i].controlled_uses;
> +  /* FIXME: introducing speuclation causes out of bounds access here.  */
> +  if (info->descriptors.length () > (unsigned)i)
> +    return info->descriptors[i].controlled_uses;
> +  return IPA_UNDESCRIBED_USE;
>  }
>  
>  /* Set the controlled counter of a given parameter.  */
> @@ -479,6 +482,7 @@ struct GTY(()) ipa_edge_args
>  {
>    /* Vector of the callsite's jump function of each parameter.  */
> Index: ipa-prop.c
> ===================================================================
> --- ipa-prop.c	(revision 215792)
> +++ ipa-prop.c	(working copy)
> @@ -2608,11 +2625,15 @@ update_jump_functions_after_inlining (st
>    for (i = 0; i < count; i++)
>      {
>        struct ipa_jump_func *dst = ipa_get_ith_jump_func (args, i);
> +      struct ipa_polymorphic_call_context *dst_ctx
> +	= ipa_get_ith_polymorhic_call_context (args, i);
>  
>        if (dst->type == IPA_JF_ANCESTOR)
>  	{
>  	  struct ipa_jump_func *src;
>  	  int dst_fid = dst->value.ancestor.formal_id;
> +	  struct ipa_polymorphic_call_context *src_ctx
> +	    = ipa_get_ith_polymorhic_call_context (top, dst_fid);

This should be moved down below the check that there is not a mismatch
between number of formal parameters and actual arguments, to the same
place where we initialize "src."

>  
>  	  /* Variable number of arguments can cause havoc if we try to access
>  	     one that does not exist in the inlined edge.  So make sure we
> @@ -2625,6 +2646,22 @@ update_jump_functions_after_inlining (st
>  
>  	  src = ipa_get_ith_jump_func (top, dst_fid);
>  
> +	  if (src_ctx && !src_ctx->useless_p ())
> +	    {
> +	      struct ipa_polymorphic_call_context ctx = *src_ctx;
> +
> +	      /* TODO: Make type preserved safe WRT contexts.  */
> +	      if (!dst->value.ancestor.agg_preserved)
> +		ctx.make_speculative ();
> +	      ctx.offset_by (dst->value.ancestor.offset);
> +	      if (!ctx.useless_p ())
> +		{
> +		  vec_safe_grow_cleared (args->polymorphic_call_contexts,
> +					 count);
> +		  dst_ctx = ipa_get_ith_polymorhic_call_context (args, i);
> +		}
> +	    }

I believe that dst_ctx->combine_with (ctx) is missing here?

Thanks,

Martin



More information about the Gcc-patches mailing list