Re: [GOOGLE] LIPO resolved node handling for inline clone references

2014-07-01 Thread Xinliang David Li
Ok, with minor fix to the comments:

1) Before possibly cloning --> Before possibly creating a new
2) It is not clear from the comments that the remaining references are
from call statements in ipa cp clones that have not been fixed up.
Possibly add a 3 line example to show the call statements calling the
original function --> after fixup it will be calling the callee's
clone?

thanks,

David

On Tue, Jul 1, 2014 at 7:06 AM, Teresa Johnson  wrote:
> This patch addresses an issue with looking up the resolved node
> rebuilding cgraph references during clone materialization. It is
> possible that in ipa-cp cases the resolved node was eliminated during
> inlining and indirect call promotion, but the clone still exists as a
> function call argument in an inline virtual clone. The clones would be
> removed later when we actually do the inline transformation, but in
> the meantime during clone materialization the cgraph references are
> rebuilt, and the temporary reference on the call still exists in the
> IR. Handle this by skipping such references in inline clones.
>
> Passes regression tests. Ok for google/4_9?
>
> Teresa
>
> 2014-07-01  Teresa Johnson  
>
> Google ref b/15411384.
> * cgraphbuild.c (mark_address): Skip resolved node lookup
> in inline copies.
>
> Index: cgraphbuild.c
> ===
> --- cgraphbuild.c   (revision 211957)
> +++ cgraphbuild.c   (working copy)
> @@ -389,8 +389,28 @@ mark_address (gimple stmt, tree addr, tree, void *
>addr = get_base_address (addr);
>if (TREE_CODE (addr) == FUNCTION_DECL)
>  {
> +  /* Before possibly cloning the node in cgraph_get_create_node,
> + save the current cgraph node for addr.  */
> +  struct cgraph_node *first_clone = cgraph_get_node (addr);
>struct cgraph_node *node = cgraph_get_create_node (addr);
> -  if (L_IPO_COMP_MODE && cgraph_pre_profiling_inlining_done)
> +  /* In LIPO mode we use the resolved node. However, there is
> + a possibility that it may not exist at this point. This
> + can happen in cases of ipa-cp, where this is a reference
> + that will eventually go away during inline_transform when we
> + invoke cgraph_redirect_edge_call_stmt_to_callee to rewrite
> + the call_stmt and skip some arguments. It is possible
> + that earlier during inline_call the references to the original
> + non-cloned resolved node were all eliminated, and it was removed.
> + However, virtual clones may stick around until inline_transform,
> + due to references in other virtual clones, at which point they
> + will all be removed. In between inline_call and inline_transform,
> + however, we will materialize clones which would rebuild references
> + and end up here upon still seeing the reference on the call.
> + Handle this by skipping the resolved node lookup when the first
> + clone was marked global.inlined_to (i.e. it is a virtual clone,
> + the original is gone).  */
> +  if (L_IPO_COMP_MODE && cgraph_pre_profiling_inlining_done
> +  && first_clone && !first_clone->global.inlined_to)
>  node = cgraph_lipo_get_resolved_node (addr);
>
>cgraph_mark_address_taken_node (node);
>
>
> --
> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413


[GOOGLE] LIPO resolved node handling for inline clone references

2014-07-01 Thread Teresa Johnson
This patch addresses an issue with looking up the resolved node
rebuilding cgraph references during clone materialization. It is
possible that in ipa-cp cases the resolved node was eliminated during
inlining and indirect call promotion, but the clone still exists as a
function call argument in an inline virtual clone. The clones would be
removed later when we actually do the inline transformation, but in
the meantime during clone materialization the cgraph references are
rebuilt, and the temporary reference on the call still exists in the
IR. Handle this by skipping such references in inline clones.

Passes regression tests. Ok for google/4_9?

Teresa

2014-07-01  Teresa Johnson  

Google ref b/15411384.
* cgraphbuild.c (mark_address): Skip resolved node lookup
in inline copies.

Index: cgraphbuild.c
===
--- cgraphbuild.c   (revision 211957)
+++ cgraphbuild.c   (working copy)
@@ -389,8 +389,28 @@ mark_address (gimple stmt, tree addr, tree, void *
   addr = get_base_address (addr);
   if (TREE_CODE (addr) == FUNCTION_DECL)
 {
+  /* Before possibly cloning the node in cgraph_get_create_node,
+ save the current cgraph node for addr.  */
+  struct cgraph_node *first_clone = cgraph_get_node (addr);
   struct cgraph_node *node = cgraph_get_create_node (addr);
-  if (L_IPO_COMP_MODE && cgraph_pre_profiling_inlining_done)
+  /* In LIPO mode we use the resolved node. However, there is
+ a possibility that it may not exist at this point. This
+ can happen in cases of ipa-cp, where this is a reference
+ that will eventually go away during inline_transform when we
+ invoke cgraph_redirect_edge_call_stmt_to_callee to rewrite
+ the call_stmt and skip some arguments. It is possible
+ that earlier during inline_call the references to the original
+ non-cloned resolved node were all eliminated, and it was removed.
+ However, virtual clones may stick around until inline_transform,
+ due to references in other virtual clones, at which point they
+ will all be removed. In between inline_call and inline_transform,
+ however, we will materialize clones which would rebuild references
+ and end up here upon still seeing the reference on the call.
+ Handle this by skipping the resolved node lookup when the first
+ clone was marked global.inlined_to (i.e. it is a virtual clone,
+ the original is gone).  */
+  if (L_IPO_COMP_MODE && cgraph_pre_profiling_inlining_done
+  && first_clone && !first_clone->global.inlined_to)
 node = cgraph_lipo_get_resolved_node (addr);

   cgraph_mark_address_taken_node (node);


-- 
Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413