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
