unofficial mirror of bug-guile@gnu.org 
 help / color / mirror / Atom feed
From: Ian Price <ianprice90@googlemail.com>
To: Mark H Weaver <mhw@netris.org>
Cc: 15533@debbugs.gnu.org
Subject: bug#15533: optimizing away noticeable effects
Date: Tue, 08 Oct 2013 18:13:05 +0100	[thread overview]
Message-ID: <8738obd6u6.fsf@Kagami.home> (raw)
In-Reply-To: <877gdpc96f.fsf@Kagami.home> (Ian Price's message of "Mon, 07 Oct 2013 17:55:36 +0100")

Ian Price <ianprice90@googlemail.com> writes:

> Does it make sense to add it to find-definition? or should we add it
> before the use in that case?

I've decided that it does, and I've made the following (tentative)
change on my own guile install.

         (cond
          ((lookup (lexical-ref-gensym x))
           => (lambda (op)
-               (let ((y (or (operand-residual-value op)
-                            (visit-operand op counter 'value 10 10)
-                            (operand-source op))))
-                 (cond
-                  ((and (lexical-ref? y)
-                        (= (lexical-refcount (lexical-ref-gensym x)) 1))
-                   ;; X is a simple alias for Y.  Recurse, regardless of
-                   ;; the number of aliases we were expecting.
-                   (find-definition y n-aliases))
-                  ((= (lexical-refcount (lexical-ref-gensym x)) n-aliases)
-                   ;; We found a definition that is aliased the right
-                   ;; number of times.  We still recurse in case it is a
-                   ;; lexical.
-                   (values (find-definition y 1)
-                           op))
-                  (else
-                   ;; We can't account for our aliases.
-                   (values #f #f))))))
+               (if (var-set? (operand-var op))
+                   (values #f #f)
+                   
+                 ;; var-set? (operand-var ) => #f #f ?
+                 (let ((y (or (operand-residual-value op)
+                              (visit-operand op counter 'value 10 10)
+                              (operand-source op))))
+                   (cond
+                    ((and (lexical-ref? y)
+                          (= (lexical-refcount (lexical-ref-gensym x)) 1))
+                     ;; X is a simple alias for Y.  Recurse, regardless of
+                     ;; the number of aliases we were expecting.
+                     (find-definition y n-aliases))
+                    ((= (lexical-refcount (lexical-ref-gensym x)) n-aliases)
+                     ;; We found a definition that is aliased the right
+                     ;; number of times.  We still recurse in case it is a
+                     ;; lexical.
+                     (values (find-definition y 1)
+                             op))
+                    (else
+                     ;; We can't account for our aliases.
+                     (values #f #f)))))))

It's a little invasive because of the 'if', but the meat of it is

+               (if (var-set? (operand-var op))
+                   (values #f #f)

The check for mutability needs to come before the let, since that's
where we do the lookup for a value, so it would be too late.

If Andy is happy with this change, I'll add a test, and push a commit,
but I'm going leave it to his discretion.

-- 
Ian Price -- shift-reset.com

"Programming is like pinball. The reward for doing it well is
the opportunity to do it again" - from "The Wizardy Compiled"





  reply	other threads:[~2013-10-08 17:13 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-05 19:27 bug#15533: optimizing away noticeable effects Ian Price
2013-10-05 20:28 ` Mark H Weaver
2013-10-05 20:45   ` Mark H Weaver
2013-10-06  6:39     ` Mark H Weaver
2013-10-06  7:36       ` Mark H Weaver
2013-10-07 16:55         ` Ian Price
2013-10-08 17:13           ` Ian Price [this message]
2013-10-23 10:16             ` Ian Price
2014-01-07  4:38               ` Ian Price

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

  List information: https://www.gnu.org/software/guile/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8738obd6u6.fsf@Kagami.home \
    --to=ianprice90@googlemail.com \
    --cc=15533@debbugs.gnu.org \
    --cc=mhw@netris.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.
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).