From: Stefan Monnier <monnier@IRO.UMontreal.CA>
To: Chris Gray <chrismgray@gmail.com>
Cc: emacs-devel@gnu.org
Subject: Re: tail-call elimination
Date: Mon, 07 Jan 2013 13:28:15 -0500 [thread overview]
Message-ID: <jwvwqvonadj.fsf-monnier+emacs@gnu.org> (raw)
In-Reply-To: <87sj6m3x5c.fsf@stuffy.starviewtechnology.com> (Chris Gray's message of "Mon, 31 Dec 2012 11:16:15 -0700")
> 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
prev parent reply other threads:[~2013-01-07 18:28 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-12-11 2:57 tail-call elimination Chris Gray
2012-12-11 3:17 ` Stefan Monnier
2012-12-11 6:13 ` Daniel Colascione
2012-12-11 6:45 ` Chris Gray
2012-12-11 13:34 ` Stefan Monnier
2012-12-11 14:30 ` Wolfgang Jenkner
2012-12-11 15:13 ` Stefan Monnier
2012-12-31 18:16 ` Chris Gray
2013-01-07 18:28 ` Stefan Monnier [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=jwvwqvonadj.fsf-monnier+emacs@gnu.org \
--to=monnier@iro.umontreal.ca \
--cc=chrismgray@gmail.com \
--cc=emacs-devel@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.