all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
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



      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.