| Message ID | alpine.LNX.2.00.1206201148060.5191@zhemvz.fhfr.qr |
|---|---|
| State | New |
| Headers | show |
> > Tailcalls have no argument setup cost and no return value cost. > This patch adjusts estminate_num_insns to reflect that. > > Honza, does this look correct? > > Bootstrapped and tested on x86_64-unknown-linux-gnu. > > Thanks, > Richard. > > 2012-06-20 Richard Guenther <rguenther@suse.de> > > * tree-inline.c (estimate_num_insns): Estimate call cost for > tailcalls properly. Well, as discussed offline, this change should currently be no-op since we discover tail calls only very late in the game. I am not sure I agree that the argument costs are zeroed out. I.e. the arguments are generally passed the same way as they would be for normal call, just they are homed at different place of the stack frame. (note that we mark by the tail call flag far more calls than those that are really expanded to tailcall because target limitations are checked only in calls.c). Finally ipa-cp use estimate_move cost to estimate savings for propagating and I think there is risk in arriving to negative numbers when costs are not accounted at all calls. So I am not sure we want to keep the patch in mainline in the current form... Honza
On Sat, 23 Jun 2012, Jan Hubicka wrote: > > > > Tailcalls have no argument setup cost and no return value cost. > > This patch adjusts estminate_num_insns to reflect that. > > > > Honza, does this look correct? > > > > Bootstrapped and tested on x86_64-unknown-linux-gnu. > > > > Thanks, > > Richard. > > > > 2012-06-20 Richard Guenther <rguenther@suse.de> > > > > * tree-inline.c (estimate_num_insns): Estimate call cost for > > tailcalls properly. > > Well, as discussed offline, this change should currently be no-op since > we discover tail calls only very late in the game. > > I am not sure I agree that the argument costs are zeroed out. I.e. the > arguments are generally passed the same way as they would be for normal call, > just they are homed at different place of the stack frame. > (note that we mark by the tail call flag far more calls than those that > are really expanded to tailcall because target limitations are checked only > in calls.c). > > Finally ipa-cp use estimate_move cost to estimate savings for propagating > and I think there is risk in arriving to negative numbers when costs > are not accounted at all calls. > > So I am not sure we want to keep the patch in mainline in the current form... I have reverted the patch. Richard.
Index: gcc/tree-inline.c =================================================================== --- gcc/tree-inline.c (revision 188817) +++ gcc/tree-inline.c (working copy) @@ -3611,12 +3611,15 @@ estimate_num_insns (gimple stmt, eni_wei } cost = node ? weights->call_cost : weights->indirect_call_cost; - if (gimple_call_lhs (stmt)) - cost += estimate_move_cost (TREE_TYPE (gimple_call_lhs (stmt))); - for (i = 0; i < gimple_call_num_args (stmt); i++) + if (!gimple_call_tail_p (stmt)) { - tree arg = gimple_call_arg (stmt, i); - cost += estimate_move_cost (TREE_TYPE (arg)); + if (gimple_call_lhs (stmt)) + cost += estimate_move_cost (TREE_TYPE (gimple_call_lhs (stmt))); + for (i = 0; i < gimple_call_num_args (stmt); i++) + { + tree arg = gimple_call_arg (stmt, i); + cost += estimate_move_cost (TREE_TYPE (arg)); + } } break; }