public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Adjust call stmt cost for tailcalls
@ 2012-06-20  9:55 Richard Guenther
  2012-06-23  4:46 ` Jan Hubicka
  0 siblings, 1 reply; 3+ messages in thread
From: Richard Guenther @ 2012-06-20  9:55 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jan Hubicka


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.

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;
       }

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Adjust call stmt cost for tailcalls
  2012-06-20  9:55 [PATCH] Adjust call stmt cost for tailcalls Richard Guenther
@ 2012-06-23  4:46 ` Jan Hubicka
  2012-06-26 13:41   ` Richard Guenther
  0 siblings, 1 reply; 3+ messages in thread
From: Jan Hubicka @ 2012-06-23  4:46 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches, Jan Hubicka

> 
> 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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Adjust call stmt cost for tailcalls
  2012-06-23  4:46 ` Jan Hubicka
@ 2012-06-26 13:41   ` Richard Guenther
  0 siblings, 0 replies; 3+ messages in thread
From: Richard Guenther @ 2012-06-26 13:41 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: gcc-patches, Jan Hubicka

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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2012-06-26 13:09 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-20  9:55 [PATCH] Adjust call stmt cost for tailcalls Richard Guenther
2012-06-23  4:46 ` Jan Hubicka
2012-06-26 13:41   ` Richard Guenther

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).