Looks good to me.
David
On Thu, Apr 10, 2014 at 2:04 PM, Teresa Johnson <tejohn...@google.com> wrote:
> This patch fixes an issue where self-recursive calls were missed
> during ipa inlining in LIPO compiles, since the resolved nodes were
> not checked. When a self-recursive call was inlined incorrectly,
> ipa_inline redirected the edge to the resolved node, leading to
> a cycle in the cgraph that wasn't expected and resulted in an
> infinite loop in estimate_calls_size_and_time.
>
> Also fix an inconsistency in the declaration of include_all_aux
> that was exposed when l-ipo.h was included in cgraph.
>
> Google ref b/13912450.
>
> Bootstrapped and tested. Ok for google/4_8?
>
> Thanks,
> Teresa
>
> 2014-04-10 Teresa Johnson <tejohn...@google.com>
>
> * cgraph.h (cgraph_edge_recursive_p): Check the resolved
> node in LIPO compiles.
> * l-ipo.h (include_all_aux): Fix declaration that didn't
> match definition.
> * Makefile.in (CGRAPH_H): Include l-ipo.h.
>
> Index: cgraph.h
> ===================================================================
> --- cgraph.h (revision 209280)
> +++ cgraph.h (working copy)
> @@ -28,6 +28,7 @@ along with GCC; see the file COPYING3. If not see
> #include "basic-block.h"
> #include "function.h"
> #include "ipa-ref.h"
> +#include "l-ipo.h"
>
> /* Symbol table consists of functions and variables.
> TODO: add labels, constant pool and aliases. */
> @@ -1388,10 +1389,15 @@ static inline bool
> cgraph_edge_recursive_p (struct cgraph_edge *e)
> {
> struct cgraph_node *callee = cgraph_function_or_thunk_node (e->callee,
> NULL);
> + struct cgraph_node *caller = e->caller;
> if (e->caller->global.inlined_to)
> - return e->caller->global.inlined_to->symbol.decl == callee->symbol.decl;
> - else
> - return e->caller->symbol.decl == callee->symbol.decl;
> + caller = e->caller->global.inlined_to;
> + if (L_IPO_COMP_MODE && cgraph_pre_profiling_inlining_done)
> + {
> + callee = cgraph_lipo_get_resolved_node (callee->symbol.decl);
> + caller = cgraph_lipo_get_resolved_node (caller->symbol.decl);
> + }
> + return (caller->symbol.decl == callee->symbol.decl);
> }
>
> /* Return true if the TM_CLONE bit is set for a given FNDECL. */
> Index: l-ipo.h
> ===================================================================
> --- l-ipo.h (revision 209280)
> +++ l-ipo.h (working copy)
> @@ -44,7 +44,7 @@ extern unsigned primary_module_id;
>
> /* Current module id. */
> extern unsigned current_module_id;
> -extern unsigned include_all_aux;
> +extern bool include_all_aux;
> extern struct gcov_module_info **module_infos;
> extern int is_last_module (unsigned mod_id);
>
> Index: Makefile.in
> ===================================================================
> --- Makefile.in (revision 209280)
> +++ Makefile.in (working copy)
> @@ -904,7 +904,8 @@ CFGLOOP_H = cfgloop.h $(BASIC_BLOCK_H) double-int.
> IPA_UTILS_H = ipa-utils.h $(TREE_H) $(CGRAPH_H)
> IPA_REFERENCE_H = ipa-reference.h $(BITMAP_H) $(TREE_H)
> CGRAPH_H = cgraph.h $(VEC_H) $(TREE_H) $(BASIC_BLOCK_H) $(FUNCTION_H) \
> - cif-code.def ipa-ref.h ipa-ref-inline.h $(LINKER_PLUGIN_API_H) is-a.h
> + cif-code.def ipa-ref.h ipa-ref-inline.h $(LINKER_PLUGIN_API_H) is-a.h
> \
> + l-ipo.h
> DF_H = df.h $(BITMAP_H) $(REGSET_H) sbitmap.h $(BASIC_BLOCK_H) \
> alloc-pool.h $(TIMEVAR_H)
> VALTRACK_H = valtrack.h $(BITMAP_H) $(DF_H) $(RTL_H) $(BASIC_BLOCK_H) \
>
>
> --
> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
