Re: [PATCH] Account for devirtualization opportunities in inliner

[Jan Hubicka <hubicka at ucw dot cz>]

Hi,
sorry for delayed review.  I am still trying to get ipa-inline-analysis to behave well on real codebases
and make my mind around how to get more advanced hints, like this one, into it.

diff --git a/gcc/ipa-inline-analysis.c b/gcc/ipa-inline-analysis.c
index bd4d2ea..5e88c2d 100644
--- a/gcc/ipa-inline-analysis.c
+++ b/gcc/ipa-inline-analysis.c
@@ -711,14 +711,23 @@ evaluate_conditions_for_known_args (struct cgraph_node *node,
 /* Work out what conditions might be true at invocation of E.  */
 
 static clause_t
-evaluate_conditions_for_edge (struct cgraph_edge *e, bool inline_p)
+evaluate_conditions_vals_binfos_for_edge (struct cgraph_edge *e,
+					  bool inline_p,
+					  VEC (tree, heap) **known_vals_ptr,
+					  VEC (tree, heap) **known_binfos_ptr)

Hmm, I would make clause also returned by reference to be sonsistent and perhaps
call it something like edge_properties
since it is not really only about evaulating the clause anymore.

-/* Increase SIZE and TIME for size and time needed to handle all calls in NODE.  */
+/* Estimate benefit devirtualizing indirect edge IE, provided KNOWN_VALS and
+   KNOWN_BINFOS.  */
+
+static void
+estimate_edge_devirt_benefit (struct cgraph_edge *ie,
+			      int *size, int *time, int prob,
+			      VEC (tree, heap) *known_vals,
+			      VEC (tree, heap) *known_binfos)

I think this whole logic should go into estimate_edge_time_and_size.  This way
we will save all the duplication of scaling logic
Just add the known_vals/binfos arguments.

I am not quite sure how to estimate the actual benefits.  estimate_num_insns
doesn't really make a difference in between direct and indirect calls.

I see it is good idea to inline more then the destination is known & inlinable.
This is an example when we have additional knowledge that we want to mix into
badness metric that does not directly translate to time/size.  There are multiple
cases like this.  I was thinking of adding kind of bonus metric for this purpose,
but I would suggest doing this incrementally.

What about
 1) extending estimate_num_insns wieghts to account direct calls differently
    from indirect calls (i.e. adding indirect_call cost value into eni wights)
    I would set it 2 for size metrics and 15 for time metrics for start
 2) make estimate_edge_time_and_size to subtract difference of those two metrics
    from edge costs when destination is direct.

Incrementally we can think of how to extra prioritize direct calls with inlinable
targets.
+/* Increase SIZE and TIME for size and time needed to handle all calls in NODE.
+   POSSIBLE_TRUTHS, KNOWN_VALS and KNOWN_BINFOS describe context of the call
+   site.  */
 
 static void
 estimate_calls_size_and_time (struct cgraph_node *node, int *size, int *time,
-			      clause_t possible_truths)
+			      clause_t possible_truths,
+			      VEC (tree, heap) *known_vals,
+			      VEC (tree, heap) *known_binfos)
 {
   struct cgraph_edge *e;
   for (e = node->callees; e; e = e->next_callee)
@@ -2125,25 +2207,35 @@ estimate_calls_size_and_time (struct cgraph_node *node, int *size, int *time,
 	    }
 	  else
 	    estimate_calls_size_and_time (e->callee, size, time,
-					  possible_truths);
+					  possible_truths,
+					  /* TODO: remap KNOWN_VALS and
+					     KNOWN_BINFOS to E->CALLEE
+					     parameters, and use them.  */
+					  NULL, NULL);

Remapping should not be needed here - the jump functions are merged after marking edge inline, so jump
functions in inlined functions actually reffer to the parameters of the function they are inlined to.

Honza