Thanks Honza,

On 2019/12/10 19:06, Jan Hubicka wrote:
>> Hi,
>>
>> On Tue, Dec 10 2019, Jan Hubicka wrote:
>>> Hi,
>>> I think the updating should treat self recursive edges as loops: that is
>>> calculate SUM of counts incomming edges which are not self recursive,
>>> calculate probability of self recursing and then set count as
>>> SUM * (1/(1-probability_of_recursion))
>>> we do same thing when computing counts withing loops.
>>>
>>> Martin, I woner how updating works for indirectly self recursive edges?
>>
>> It does not special-case them in any way, if that was the question.
> 
> Well, it should, summing only frequencies of edges entering the SCC is
> not going to give a good answer.
> If SCC is having single entry and forms a loop the generalization of
> algorithm above will work, but for general graphs this is hard problem.
> 
> I suppose we could special case self recurisve nodes (since they are
> most common) and apply some fixed scale for other SCCs?
> 
> Also i wonder if the updating is done in RPO so we do not account edges
> not updated yet?

I have several questions with you comments.
1.  Where is the loops counts prob calculation code please?  And I am not
quite sure about the formula SUM * (1/(1-probability_of_recursion)).
At this moment, the self recursive node graph will be transformed as below:

            orig_node<-----
               |    |------| 
               |  
  caller ---> new_node

Assuming:               
e1: caller    ->  new_node:  count1
e2: orig_node -> orig_node:  count2
e3: new_node  -> orig_node:  count2 (same as back edge when cloned)

e3 is not considered to be a self recursive edge now?
So:
SUM = count2
probability_of_recursion should be count2/(count2 + count2)
=>
orig_node.count = SUM * (1/(1-probability_of_recursion)) = count2 * 2

But the count2 of e3 is not correct as it was copied when cloned which makes
the calculation inaccurate.  And the important thing is new_node.count is
still UNCERTAIN.

2.  Also, the orig_node_count is already inaccurate due to
ipa-cp.c:4289   orig_node_count = (orig_sum + new_sum).apply_scale (12, 10);
Since the parameter param_ipa_cp_max_recursive_depth defines how many nodes
are cloned, is it reasonable to update self_scc node as below to simplify the
calculation(replace self_recursive_p with info->node_is_self_scc)?

+  class ipa_node_params *info = IPA_NODE_REF (orig_node);
+  if (info && info->node_is_self_scc)
+    new_node->count
+      = (orig_sum + new_sum).apply_scale (1, param_ipa_cp_max_recursive_depth);
+  else
+    new_node->count = new_sum;

3.  The recursive ipa-cp will create multiple SCC nodes based on orig_node,
next new_node2 no need update previous new_node1, as each new_node takes
1/param_ipa_cp_max_recursive_depth execution count from orig_node.

BR
Xiong Hu

> 
> Honza
> 
>>
>> Martin

Reply via email to