diff mbox

Fix overactive reference removal with speculative devirtualization

Message ID 20140923094048.GE14611@virgil.suse
State New
Headers show

Commit Message

Martin Jambor Sept. 23, 2014, 9:40 a.m. UTC
Hi,

code removing references to indirectly-inlined functions can remove a
few too many when speculative devirtualization gets into the picture.
Although we already handle the case when speculative devirtualization
clones an edge with a CONST jump function by also cloning the
appropriate reference, we do nothing when cloning a PASS_THROUGH jump
function.  However, inlining can later convert this into a CONST jump
function and when that gets deleted, the reference is dropped (as a
part of speculative devirtualization resolution).  Eventually this
leads to the function being removed as unreachable and missing in the
final link.  This is fixed by the following patch which increments
controlled use counter of formal parameters when this kind of edge
duplication.

I have also attempted to add a hunk to the edge removal hook that
would decrement the counter in these situation (dropping the edge
after all means the call will never happen).  However, I have run into
ordering issues within indirect inlining (and speculative
devirtualization resolution which happens as a part of making an edge
direct).  So at the moment I'm posting only this fix and will work on
the removal hook later.

Bootstrapped and tested on x86_64-linux, I have also successfully
LTO-built Firefox with the patch (when before it was failing).

Thanks,

Martin


2014-09-19  Martin Jambor  <mjambor@suse.cz>

	* ipa-prop.c (ipa_edge_duplication_hook): Update controlled_use_count
	when duplicating a PASS_THROUGH jump function when creating a
	speculative edge.

Comments

Jan Hubicka Sept. 23, 2014, 3:03 p.m. UTC | #1
> Hi,
> 
> code removing references to indirectly-inlined functions can remove a
> few too many when speculative devirtualization gets into the picture.
> Although we already handle the case when speculative devirtualization
> clones an edge with a CONST jump function by also cloning the
> appropriate reference, we do nothing when cloning a PASS_THROUGH jump
> function.  However, inlining can later convert this into a CONST jump
> function and when that gets deleted, the reference is dropped (as a
> part of speculative devirtualization resolution).  Eventually this
> leads to the function being removed as unreachable and missing in the
> final link.  This is fixed by the following patch which increments
> controlled use counter of formal parameters when this kind of edge
> duplication.
> 
> I have also attempted to add a hunk to the edge removal hook that
> would decrement the counter in these situation (dropping the edge
> after all means the call will never happen).  However, I have run into
> ordering issues within indirect inlining (and speculative
> devirtualization resolution which happens as a part of making an edge
> direct).  So at the moment I'm posting only this fix and will work on
> the removal hook later.
> 
> Bootstrapped and tested on x86_64-linux, I have also successfully
> LTO-built Firefox with the patch (when before it was failing).
> 
> Thanks,
> 
> Martin
> 
> 
> 2014-09-19  Martin Jambor  <mjambor@suse.cz>
> 
> 	* ipa-prop.c (ipa_edge_duplication_hook): Update controlled_use_count
> 	when duplicating a PASS_THROUGH jump function when creating a
> 	speculative edge.

OK, perhaps adding a comment would be good idea?
I assume you did not manage to get a self contained testcase.

Honza
> 
> diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
> index bbb417d..29c8681 100644
> --- a/gcc/ipa-prop.c
> +++ b/gcc/ipa-prop.c
> @@ -3643,6 +3643,21 @@ ipa_edge_duplication_hook (struct cgraph_edge *src, struct cgraph_edge *dst,
>  	      dst_jf->value.constant.rdesc = dst_rdesc;
>  	    }
>  	}
> +      else if (dst_jf->type == IPA_JF_PASS_THROUGH
> +	       && src->caller == dst->caller)
> +	{
> +	  struct cgraph_node *inline_root = dst->caller->global.inlined_to
> +	    ? dst->caller->global.inlined_to : dst->caller;
> +	  struct ipa_node_params *root_info = IPA_NODE_REF (inline_root);
> +	  int idx = ipa_get_jf_pass_through_formal_id (dst_jf);
> +
> +	  int c = ipa_get_controlled_uses (root_info, idx);
> +	  if (c != IPA_UNDESCRIBED_USE)
> +	    {
> +	      c++;
> +	      ipa_set_controlled_uses (root_info, idx, c);
> +	    }
> +	}
>      }
>  }
>
diff mbox

Patch

diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index bbb417d..29c8681 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -3643,6 +3643,21 @@  ipa_edge_duplication_hook (struct cgraph_edge *src, struct cgraph_edge *dst,
 	      dst_jf->value.constant.rdesc = dst_rdesc;
 	    }
 	}
+      else if (dst_jf->type == IPA_JF_PASS_THROUGH
+	       && src->caller == dst->caller)
+	{
+	  struct cgraph_node *inline_root = dst->caller->global.inlined_to
+	    ? dst->caller->global.inlined_to : dst->caller;
+	  struct ipa_node_params *root_info = IPA_NODE_REF (inline_root);
+	  int idx = ipa_get_jf_pass_through_formal_id (dst_jf);
+
+	  int c = ipa_get_controlled_uses (root_info, idx);
+	  if (c != IPA_UNDESCRIBED_USE)
+	    {
+	      c++;
+	      ipa_set_controlled_uses (root_info, idx, c);
+	    }
+	}
     }
 }