From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: tail-call elimination Date: Mon, 07 Jan 2013 13:28:15 -0500 Message-ID: References: <50C6CF11.2000706@dancol.org> <87sj6m3x5c.fsf@stuffy.starviewtechnology.com> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1357583313 4791 80.91.229.3 (7 Jan 2013 18:28:33 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Mon, 7 Jan 2013 18:28:33 +0000 (UTC) Cc: emacs-devel@gnu.org To: Chris Gray Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Mon Jan 07 19:28:48 2013 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1TsHR8-0005d5-9K for ged-emacs-devel@m.gmane.org; Mon, 07 Jan 2013 19:28:38 +0100 Original-Received: from localhost ([::1]:39106 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TsHQs-0007oR-N1 for ged-emacs-devel@m.gmane.org; Mon, 07 Jan 2013 13:28:22 -0500 Original-Received: from eggs.gnu.org ([208.118.235.92]:52729) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TsHQp-0007n9-9h for emacs-devel@gnu.org; Mon, 07 Jan 2013 13:28:20 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TsHQn-0001Qx-Tk for emacs-devel@gnu.org; Mon, 07 Jan 2013 13:28:19 -0500 Original-Received: from chene.dit.umontreal.ca ([132.204.246.20]:52299) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TsHQn-0001QG-NZ for emacs-devel@gnu.org; Mon, 07 Jan 2013 13:28:17 -0500 Original-Received: from faina.iro.umontreal.ca (lechon.iro.umontreal.ca [132.204.27.242]) by chene.dit.umontreal.ca (8.14.1/8.14.1) with ESMTP id r07ISF9U010150; Mon, 7 Jan 2013 13:28:15 -0500 Original-Received: by faina.iro.umontreal.ca (Postfix, from userid 20848) id 70943B4368; Mon, 7 Jan 2013 13:28:15 -0500 (EST) In-Reply-To: <87sj6m3x5c.fsf@stuffy.starviewtechnology.com> (Chris Gray's message of "Mon, 31 Dec 2012 11:16:15 -0700") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3.50 (gnu/linux) X-NAI-Spam-Flag: NO X-NAI-Spam-Threshold: 5 X-NAI-Spam-Score: 0 X-NAI-Spam-Rules: 1 Rules triggered RV4454=0 X-NAI-Spam-Version: 2.2.0.9309 : core <4454> : streams <887356> : uri <1313268> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 132.204.246.20 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:156122 Archived-At: > Based on feedback by Stefan, I have updated the patch. This is a > slightly less "pure" version of TCO than the original patch, but it > should be faster. (And really, the difference between this and the > "pure" version is pretty much academic.) It's looking pretty good. I have a few remaining questions/comments/nitpicks: > + stack.next = byte_stack_list; > + byte_stack_list = &stack; > + > CHECK_STRING (bytestr); > CHECK_VECTOR (vector); > CHECK_NATNUM (maxdepth); IIUC These checks are "runtime redundant" with the checks you added right after the "tail_call:" label. Try to restructure the code to remove this redundancy. > + /* If the next op is return, maybe we can eliminate the tail call */ Please add a "." at the end of that sentence and make sure it is followed by 2 spaces. > + if (SYMBOLP (fun) && !EQ (fun, Qunbound) > + && (fun = XSYMBOL (fun)->function, SYMBOLP (fun))) You can drop the !EQ (fun, Qunbound) test. > + fun = indirect_function (fun); > + if (COMPILEDP(fun)) Please add a space between COMPILEDP and the following open parenthesis. > + int prev_maxdepth = XFASTINT(maxdepth); Add the same space here too. > + args_template = syms_left; Not sure why you named the var "syms_left". > + else > + { > + top = bottom; > + } You don't need that braces (tho they don't hurt), but you do need to add a comment explaining that that elements that were on top are now referenced by `args' and will be moved back to `top' and that this move needs to be careful not to overwrite data before it reads it since the two areas may (and often will) overlap. One more thing: have you performed some performance tests? I'd recommend you compare something like cd lisp; rm **/*.elc; time make compile -- Stefan