Patchwork inline-analysis improvement

login
register
mail settings
Submitter Jan Hubicka
Date Sept. 20, 2011, 10:26 a.m.
Message ID <20110920102645.GA21804@kam.mff.cuni.cz>
Download mbox | patch
Permalink /patch/115476/
State New
Headers show

Comments

Jan Hubicka - Sept. 20, 2011, 10:26 a.m.
> > 	* ipa-inline-analysis.c (add_condition): Add conditions parameter;
> > 	simplify obviously true clauses.
> > 	(and_predicates, or_predicates): Add conditions parameter.
> > 	(inline_duplication_hoook): Update.
> > 	(mark_modified): New function.
> > 	(unmodified_parm): New function.
> > 	(eliminated_by_inlining_prob, (set_cond_stmt_execution_predicate,
> > 	set_switch_stmt_execution_predicate, will_be_nonconstant_predicate):
> > 	Use unmodified_parm.
> > 	(estimate_function_body_sizes): Update.
> > 	(remap_predicate): Update.
> 
> This breaks things in Ada:
> 
> Program received signal SIGSEGV, Segmentation fault.
> walk_aliased_vdefs_1 (ref=0xffffcbf4, vdef=0x0,
>     walker=0x873e3e0 <mark_modified>, data=0xffffcc1f, visited=0xffffcbc0,
>     cnt=0) at /home/eric/svn/gcc/gcc/tree-ssa-alias.c:1996
> 1996          gimple def_stmt = SSA_NAME_DEF_STMT (vdef);
> (gdb) bt
> #0  walk_aliased_vdefs_1 (ref=0xffffcbf4, vdef=0x0,
>     walker=0x873e3e0 <mark_modified>, data=0xffffcc1f, visited=0xffffcbc0,
>     cnt=0) at /home/eric/svn/gcc/gcc/tree-ssa-alias.c:1996
> #1  0x089c3a6d in walk_aliased_vdefs (ref=0xffffcbf4, vdef=0x0,
>     walker=0x873e3e0 <mark_modified>, data=0xffffcc1f, visited=0xffffcbc0)
>     at /home/eric/svn/gcc/gcc/tree-ssa-alias.c:2037
> #2  0x087456b5 in unmodified_parm (stmt=0xf7cf6ab0, op=0xf7cf77e0)
>     at /home/eric/svn/gcc/gcc/ipa-inline-analysis.c:1104
> #3  0x08748a99 in eliminated_by_inlining_prob (stmt=<optimized out>)
>     at /home/eric/svn/gcc/gcc/ipa-inline-analysis.c:1165

Hi,
the problem here is that we consider &param.foo to be a memory reference, while it is not.
The following patch should fix it, but because it changes the behaviour of inline heuristics,
I will run bechmarks tonight before committing it.
Eric Botcazou - Sept. 20, 2011, 7:47 p.m.
> the problem here is that we consider &param.foo to be a memory reference,
> while it is not. The following patch should fix it, but because it changes
> the behaviour of inline heuristics, I will run bechmarks tonight before
> committing it.

Thanks in advance.  Can you add PR tree-optimization/50433 to the ChangeLog?
Jan Hubicka - Sept. 21, 2011, 1:12 p.m.
> > the problem here is that we consider &param.foo to be a memory reference,
> > while it is not. The following patch should fix it, but because it changes
> > the behaviour of inline heuristics, I will run bechmarks tonight before
> > committing it.
> 
> Thanks in advance.  Can you add PR tree-optimization/50433 to the ChangeLog?

The tests tonight shows quite good results - there are quite consistent code size
reductions on C++ code w/o slowdowns so I've comitted the patch.
Sadly tramp3d does not build and the heuristics was implemented with tramp3d in mind.
Important behaviour change is that the heuristic now matches only for parameters
passed by value not for parameters passed by reference. I however have the patch for
tracking the second more consistently with aid of the new predicate infrastructure
so I guess I will fix the possible regression on tramp3d other way.

I've comitted the patch.  I never added Ada testcase, but it would make sense if you
commit it if that ICE is not covered by some other Ada test already.

Honza
Eric Botcazou - Sept. 21, 2011, 1:47 p.m.
> The tests tonight shows quite good results - there are quite consistent
> code size reductions on C++ code w/o slowdowns so I've comitted the patch.
> Sadly tramp3d does not build and the heuristics was implemented with
> tramp3d in mind. Important behaviour change is that the heuristic now
> matches only for parameters passed by value not for parameters passed by
> reference. I however have the patch for tracking the second more
> consistently with aid of the new predicate infrastructure so I guess I will
> fix the possible regression on tramp3d other way.

Thanks for the explanation and the fix.

> I've comitted the patch.  I never added Ada testcase, but it would make
> sense if you commit it if that ICE is not covered by some other Ada test
> already.

There is apparently an ACATS failure on x86-64/Darwin, but I've installed the 
testcase as gnat.dg/opt19.adb in the tree.

Patch

Index: ipa-inline-analysis.c
===================================================================
--- ipa-inline-analysis.c	(revision 179003)
+++ ipa-inline-analysis.c	(working copy)
@@ -1149,18 +1149,15 @@  eliminated_by_inlining_prob (gimple stmt
 	  {
 	    tree rhs = gimple_assign_rhs1 (stmt);
             tree lhs = gimple_assign_lhs (stmt);
-	    tree inner_rhs = rhs;
-	    tree inner_lhs = lhs;
+	    tree inner_rhs = get_base_address (rhs);
+	    tree inner_lhs = get_base_address (lhs);
 	    bool rhs_free = false;
 	    bool lhs_free = false;
 
- 	    while (handled_component_p (inner_lhs)
-		   || TREE_CODE (inner_lhs) == MEM_REF)
-	      inner_lhs = TREE_OPERAND (inner_lhs, 0);
- 	    while (handled_component_p (inner_rhs)
-	           || TREE_CODE (inner_rhs) == ADDR_EXPR
-		   || TREE_CODE (inner_rhs) == MEM_REF)
-	      inner_rhs = TREE_OPERAND (inner_rhs, 0);
+	    if (!inner_rhs)
+	      inner_rhs = rhs;
+	    if (!inner_lhs)
+	      inner_lhs = lhs;
 
 	    if (unmodified_parm (stmt, inner_rhs))
 	      rhs_free = true;