unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Minor simplification in byte-opt.el
@ 2020-07-26 20:41 Stefan Monnier
  2020-07-27  9:38 ` Mattias Engdegård
  0 siblings, 1 reply; 7+ messages in thread
From: Stefan Monnier @ 2020-07-26 20:41 UTC (permalink / raw)
  To: emacs-devel

Could someone look over the patch below to confirm it's safe?

Also, as a result of this, there is only one call to
`byte-optimize-form-code-walker` left, in `byte-optimize-form`, so maybe
the two functions could be merged, tho it's probably not worth the trouble.


        Stefan


    * lisp/emacs-lisp/byte-opt.el (byte-optimize-form-code-walker):
    After inlining, use `byte-optimize-form` so the top-level function's
    optimizer (if any) is also applied.
    Simplify the pure function handling by using `byte-optimize-constant-args`.
    (byte-optimize-all-constp): Remove.
    (byte-optimize-1+, byte-optimize-1-): Remove, since `pure` already
    takes care of it.


diff --git a/lisp/emacs-lisp/byte-opt.el b/lisp/emacs-lisp/byte-opt.el
index 6f801be545..afe07da0ec 100644
--- a/lisp/emacs-lisp/byte-opt.el
+++ b/lisp/emacs-lisp/byte-opt.el
@@ -227,7 +227,7 @@ byte-compile-log-lap
 \f
 ;;; byte-compile optimizers to support inlining
 
-(put 'inline 'byte-optimizer 'byte-optimize-inline-handler)
+(put 'inline 'byte-optimizer #'byte-optimize-inline-handler)
 
 (defun byte-optimize-inline-handler (form)
   "byte-optimize-handler for the `inline' special-form."
@@ -396,7 +396,7 @@ byte-optimize-form-code-walker
 	     (if (eq newform form)
 		 ;; Some error occurred, avoid infinite recursion
 		 form
-	       (byte-optimize-form-code-walker newform for-effect))))
+	       (byte-optimize-form newform for-effect))))
 	  ((eq (car-safe fn) 'closure) form)
 	  ((memq fn '(let let*))
 	   ;; recursively enter the optimizer for the bindings and body
@@ -554,23 +554,10 @@ byte-optimize-form-code-walker
 	   ;; Otherwise, no args can be considered to be for-effect,
 	   ;; even if the called function is for-effect, because we
 	   ;; don't know anything about that function.
-	   (let ((args (mapcar #'byte-optimize-form (cdr form))))
-	     (if (and (get fn 'pure)
-		      (byte-optimize-all-constp args))
-                 (let ((arg-values (mapcar #'eval args)))
-                   (condition-case nil
-                       (list 'quote (apply fn arg-values))
-                     (error (cons fn args))))
-	       (cons fn args)))))))
-
-(defun byte-optimize-all-constp (list)
-  "Non-nil if all elements of LIST satisfy `macroexp-const-p'."
-  (let ((constant t))
-    (while (and list constant)
-      (unless (macroexp-const-p (car list))
-	(setq constant nil))
-      (setq list (cdr list)))
-    constant))
+	   (let ((form (cons fn (mapcar #'byte-optimize-form (cdr form)))))
+	     (if (get fn 'pure)
+		 (byte-optimize-constant-args form)
+	       form))))))
 
 (defun byte-optimize-form (form &optional for-effect)
   "The source-level pass of the optimizer."
@@ -747,22 +734,6 @@ byte-optimize-minus
        ((equal args (cdr form)) form)
        (t (cons '- args))))))
 
-(defun byte-optimize-1+ (form)
-  (let ((args (cdr form)))
-    (when (null (cdr args))
-      (let ((n (car args)))
-        (when (numberp n)
-          (setq form (1+ n))))))
-  form)
-
-(defun byte-optimize-1- (form)
-  (let ((args (cdr form)))
-    (when (null (cdr args))
-      (let ((n (car args)))
-        (when (numberp n)
-          (setq form (1- n))))))
-  form)
-
 (defun byte-optimize-multiply (form)
   (let* ((args (remq 1 (byte-opt--arith-reduce #'* 1 (cdr form)))))
     (cond
@@ -802,7 +773,7 @@ byte-optimize-binary-predicate
     (condition-case ()
         (list 'quote (eval form))
       (error form)))
-   (t ;; This can enable some lapcode optimizations.
+   (t ;; Moving the constant to the end can enable some lapcode optimizations.
     (list (car form) (nth 2 form) (nth 1 form)))))
 
 (defun byte-optimize-constant-args (form)
@@ -901,37 +872,34 @@ byte-optimize-concat
         form          ; No improvement.
       (cons 'concat (nreverse newargs)))))
 
-(put 'identity 'byte-optimizer 'byte-optimize-identity)
-(put 'memq 'byte-optimizer 'byte-optimize-memq)
-(put 'memql  'byte-optimizer 'byte-optimize-member)
-(put 'member 'byte-optimizer 'byte-optimize-member)
-(put 'assoc 'byte-optimizer 'byte-optimize-assoc)
-(put 'rassoc 'byte-optimizer 'byte-optimize-assoc)
-
-(put '+   'byte-optimizer 'byte-optimize-plus)
-(put '*   'byte-optimizer 'byte-optimize-multiply)
-(put '-   'byte-optimizer 'byte-optimize-minus)
-(put '/   'byte-optimizer 'byte-optimize-divide)
-(put 'max 'byte-optimizer 'byte-optimize-associative-math)
-(put 'min 'byte-optimizer 'byte-optimize-associative-math)
-
-(put '=   'byte-optimizer 'byte-optimize-binary-predicate)
-(put 'eq  'byte-optimizer 'byte-optimize-binary-predicate)
-(put 'eql   'byte-optimizer 'byte-optimize-equal)
-(put 'equal 'byte-optimizer 'byte-optimize-equal)
-(put 'string= 'byte-optimizer 'byte-optimize-binary-predicate)
-(put 'string-equal 'byte-optimizer 'byte-optimize-binary-predicate)
-
-(put '1+  'byte-optimizer 'byte-optimize-1+)
-(put '1-  'byte-optimizer 'byte-optimize-1-)
-
-(put 'concat 'byte-optimizer 'byte-optimize-concat)
+(put 'identity 'byte-optimizer #'byte-optimize-identity)
+(put 'memq 'byte-optimizer #'byte-optimize-memq)
+(put 'memql  'byte-optimizer #'byte-optimize-member)
+(put 'member 'byte-optimizer #'byte-optimize-member)
+(put 'assoc 'byte-optimizer #'byte-optimize-assoc)
+(put 'rassoc 'byte-optimizer #'byte-optimize-assoc)
+
+(put '+   'byte-optimizer #'byte-optimize-plus)
+(put '*   'byte-optimizer #'byte-optimize-multiply)
+(put '-   'byte-optimizer #'byte-optimize-minus)
+(put '/   'byte-optimizer #'byte-optimize-divide)
+(put 'max 'byte-optimizer #'byte-optimize-associative-math)
+(put 'min 'byte-optimizer #'byte-optimize-associative-math)
+
+(put '=   'byte-optimizer #'byte-optimize-binary-predicate)
+(put 'eq  'byte-optimizer #'byte-optimize-binary-predicate)
+(put 'eql   'byte-optimizer #'byte-optimize-equal)
+(put 'equal 'byte-optimizer #'byte-optimize-equal)
+(put 'string= 'byte-optimizer #'byte-optimize-binary-predicate)
+(put 'string-equal 'byte-optimizer #'byte-optimize-binary-predicate)
+
+(put 'concat 'byte-optimizer #'byte-optimize-concat)
 
 ;; I'm not convinced that this is necessary.  Doesn't the optimizer loop
 ;; take care of this? - Jamie
 ;; I think this may some times be necessary to reduce ie (quote 5) to 5,
 ;; so arithmetic optimizers recognize the numeric constant.  - Hallvard
-(put 'quote 'byte-optimizer 'byte-optimize-quote)
+(put 'quote 'byte-optimizer #'byte-optimize-quote)
 (defun byte-optimize-quote (form)
   (if (or (consp (nth 1 form))
 	  (and (symbolp (nth 1 form))
@@ -1049,16 +1017,16 @@ byte-optimize-while
   (if (nth 1 form)
       form))
 
-(put 'and   'byte-optimizer 'byte-optimize-and)
-(put 'or    'byte-optimizer 'byte-optimize-or)
-(put 'cond  'byte-optimizer 'byte-optimize-cond)
-(put 'if    'byte-optimizer 'byte-optimize-if)
-(put 'while 'byte-optimizer 'byte-optimize-while)
+(put 'and   'byte-optimizer #'byte-optimize-and)
+(put 'or    'byte-optimizer #'byte-optimize-or)
+(put 'cond  'byte-optimizer #'byte-optimize-cond)
+(put 'if    'byte-optimizer #'byte-optimize-if)
+(put 'while 'byte-optimizer #'byte-optimize-while)
 
 ;; byte-compile-negation-optimizer lives in bytecomp.el
-(put '/= 'byte-optimizer 'byte-compile-negation-optimizer)
-(put 'atom 'byte-optimizer 'byte-compile-negation-optimizer)
-(put 'nlistp 'byte-optimizer 'byte-compile-negation-optimizer)
+(put '/= 'byte-optimizer #'byte-compile-negation-optimizer)
+(put 'atom 'byte-optimizer #'byte-compile-negation-optimizer)
+(put 'nlistp 'byte-optimizer #'byte-compile-negation-optimizer)
 
 
 (defun byte-optimize-funcall (form)
@@ -1086,12 +1054,12 @@ byte-optimize-apply
 	      nil))
 	form)))
 
-(put 'funcall 'byte-optimizer 'byte-optimize-funcall)
-(put 'apply   'byte-optimizer 'byte-optimize-apply)
+(put 'funcall 'byte-optimizer #'byte-optimize-funcall)
+(put 'apply   'byte-optimizer #'byte-optimize-apply)
 
 
-(put 'let 'byte-optimizer 'byte-optimize-letX)
-(put 'let* 'byte-optimizer 'byte-optimize-letX)
+(put 'let 'byte-optimizer #'byte-optimize-letX)
+(put 'let* 'byte-optimizer #'byte-optimize-letX)
 (defun byte-optimize-letX (form)
   (cond ((null (nth 1 form))
 	 ;; No bindings
@@ -1107,7 +1075,7 @@ byte-optimize-letX
 	   (list 'let* (reverse (cdr binds)) (nth 1 (car binds)) nil)))))
 
 
-(put 'nth 'byte-optimizer 'byte-optimize-nth)
+(put 'nth 'byte-optimizer #'byte-optimize-nth)
 (defun byte-optimize-nth (form)
   (if (= (safe-length form) 3)
       (if (memq (nth 1 form) '(0 1))
@@ -1117,7 +1085,7 @@ byte-optimize-nth
 	form)
     form))
 
-(put 'nthcdr 'byte-optimizer 'byte-optimize-nthcdr)
+(put 'nthcdr 'byte-optimizer #'byte-optimize-nthcdr)
 (defun byte-optimize-nthcdr (form)
   (if (= (safe-length form) 3)
       (if (memq (nth 1 form) '(0 1 2))
@@ -1133,7 +1101,7 @@ byte-optimize-nthcdr
 ;; optimize string-as-unibyte, string-as-multibyte, string-make-unibyte,
 ;; string-make-multibyte for constant args.
 
-(put 'set 'byte-optimizer 'byte-optimize-set)
+(put 'set 'byte-optimizer #'byte-optimize-set)
 (defun byte-optimize-set (form)
   (let ((var (car-safe (cdr-safe form))))
     (cond




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

* Re: Minor simplification in byte-opt.el
  2020-07-26 20:41 Minor simplification in byte-opt.el Stefan Monnier
@ 2020-07-27  9:38 ` Mattias Engdegård
  2020-07-27 14:24   ` Stefan Monnier
  0 siblings, 1 reply; 7+ messages in thread
From: Mattias Engdegård @ 2020-07-27  9:38 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 611 bytes --]

26 juli 2020 kl. 22.41 skrev Stefan Monnier <monnier@iro.umontreal.ca>:

> Could someone look over the patch below to confirm it's safe?

Thank you, looks decent to me. Good that you spotted the 1+/1-, I forgot about those.

> Also, as a result of this, there is only one call to
> `byte-optimize-form-code-walker` left, in `byte-optimize-form`, so maybe
> the two functions could be merged, tho it's probably not worth the trouble.

Maybe -- there seems to be little to gain from integrating them other than the elimination of the function call.

By the way, what about this little improvement?


[-- Attachment #2: 0001-Simplify-and-streamline-optimizer-clauses.patch --]
[-- Type: application/octet-stream, Size: 2507 bytes --]

From cc6cbb304193c62354d3cb6b0b76f8ce066b2189 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Mon, 27 Jul 2020 11:27:00 +0200
Subject: [PATCH] Simplify and streamline optimizer clauses

* lisp/emacs-lisp/byte-opt.el (byte-optimize-form-code-walker):
Remove clause for 'with-output-to-temp-buffer', since it is a
macro and will have been expanded before reaching this point.
Move clauses for 'lambda' and 'closure' to avoid splitting
a cond jump table.
---
 lisp/emacs-lisp/byte-opt.el | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/lisp/emacs-lisp/byte-opt.el b/lisp/emacs-lisp/byte-opt.el
index 6f801be545..48efff911f 100644
--- a/lisp/emacs-lisp/byte-opt.el
+++ b/lisp/emacs-lisp/byte-opt.el
@@ -391,13 +391,6 @@ byte-optimize-form-code-walker
 	   (and (nth 1 form)
 		(not for-effect)
 		form))
-	  ((eq (car-safe fn) 'lambda)
-	   (let ((newform (byte-compile-unfold-lambda form)))
-	     (if (eq newform form)
-		 ;; Some error occurred, avoid infinite recursion
-		 form
-	       (byte-optimize-form-code-walker newform for-effect))))
-	  ((eq (car-safe fn) 'closure) form)
 	  ((memq fn '(let let*))
 	   ;; recursively enter the optimizer for the bindings and body
 	   ;; of a let or let*.  This for depth-firstness: forms that
@@ -444,13 +437,6 @@ byte-optimize-form-code-walker
 	   ;; will be optimized away in the lap-optimize pass.
 	   (cons fn (byte-optimize-body (cdr form) for-effect)))
 
-	  ((eq fn 'with-output-to-temp-buffer)
-	   ;; this is just like the above, except for the first argument.
-	   (cons fn
-	     (cons
-	      (byte-optimize-form (nth 1 form) nil)
-	      (byte-optimize-body (cdr (cdr form)) for-effect))))
-
 	  ((eq fn 'if)
 	   (when (< (length form) 3)
 	     (byte-compile-warn "too few arguments for `if'"))
@@ -530,6 +516,15 @@ byte-optimize-form-code-walker
           ;; Needed as long as we run byte-optimize-form after cconv.
           ((eq fn 'internal-make-closure) form)
 
+	  ((eq (car-safe fn) 'lambda)
+	   (let ((newform (byte-compile-unfold-lambda form)))
+	     (if (eq newform form)
+		 ;; Some error occurred, avoid infinite recursion
+		 form
+	       (byte-optimize-form-code-walker newform for-effect))))
+
+	  ((eq (car-safe fn) 'closure) form)
+
           ((byte-code-function-p fn)
            (cons fn (mapcar #'byte-optimize-form (cdr form))))
 
-- 
2.21.1 (Apple Git-122.3)


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

* Re: Minor simplification in byte-opt.el
  2020-07-27  9:38 ` Mattias Engdegård
@ 2020-07-27 14:24   ` Stefan Monnier
  2020-07-28 14:43     ` Mattias Engdegård
  0 siblings, 1 reply; 7+ messages in thread
From: Stefan Monnier @ 2020-07-27 14:24 UTC (permalink / raw)
  To: Mattias Engdegård; +Cc: emacs-devel

>> Could someone look over the patch below to confirm it's safe?
> Thank you, looks decent to me. Good that you spotted the 1+/1-, I forgot about those.

Thanks.

>> Also, as a result of this, there is only one call to
>> `byte-optimize-form-code-walker` left, in `byte-optimize-form`, so maybe
>> the two functions could be merged, tho it's probably not worth the trouble.
> Maybe -- there seems to be little to gain from integrating them other than
> the elimination of the function call.

Exactly :-(

> By the way, what about this little improvement?

Looks good [ tho IMO we shouldn't support `closure` there.  ]


        Stefan




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

* Re: Minor simplification in byte-opt.el
  2020-07-27 14:24   ` Stefan Monnier
@ 2020-07-28 14:43     ` Mattias Engdegård
  2020-07-28 14:53       ` Stefan Monnier
  0 siblings, 1 reply; 7+ messages in thread
From: Mattias Engdegård @ 2020-07-28 14:43 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

27 juli 2020 kl. 16.24 skrev Stefan Monnier <monnier@iro.umontreal.ca>:

> [ tho IMO we shouldn't support `closure` there.  ]

Probably. I was a bit unsure about whether that case could ever occur and didn't want to change too many things at once.




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

* Re: Minor simplification in byte-opt.el
  2020-07-28 14:43     ` Mattias Engdegård
@ 2020-07-28 14:53       ` Stefan Monnier
  2020-07-28 15:18         ` Mattias Engdegård
  0 siblings, 1 reply; 7+ messages in thread
From: Stefan Monnier @ 2020-07-28 14:53 UTC (permalink / raw)
  To: Mattias Engdegård; +Cc: emacs-devel

>> [ tho IMO we shouldn't support `closure` there.  ]
> Probably. I was a bit unsure about whether that case could ever occur and
> didn't want to change too many things at once.

We currently do allow it (in the interpreter and the byte-compiler), but
I think this is a mistake and we should emit a warning (if not an error)
when the byte-compiler encounters such a construct.


        Stefan




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

* Re: Minor simplification in byte-opt.el
  2020-07-28 14:53       ` Stefan Monnier
@ 2020-07-28 15:18         ` Mattias Engdegård
  2020-07-28 16:08           ` Stefan Monnier
  0 siblings, 1 reply; 7+ messages in thread
From: Mattias Engdegård @ 2020-07-28 15:18 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

28 juli 2020 kl. 16.53 skrev Stefan Monnier <monnier@iro.umontreal.ca>:

> We currently do allow it (in the interpreter and the byte-compiler), but
> I think this is a mistake and we should emit a warning (if not an error)
> when the byte-compiler encounters such a construct.

Yes, sorry, I meant that I wasn't sure whether could be synthesised (by cconv) without the user actually ever feeding an explicit 'closure' construct into the compiler. After all, 'closure' isn't really in the language, is it?




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

* Re: Minor simplification in byte-opt.el
  2020-07-28 15:18         ` Mattias Engdegård
@ 2020-07-28 16:08           ` Stefan Monnier
  0 siblings, 0 replies; 7+ messages in thread
From: Stefan Monnier @ 2020-07-28 16:08 UTC (permalink / raw)
  To: Mattias Engdegård; +Cc: emacs-devel

>> We currently do allow it (in the interpreter and the byte-compiler), but
>> I think this is a mistake and we should emit a warning (if not an error)
>> when the byte-compiler encounters such a construct.
>
> Yes, sorry, I meant that I wasn't sure whether could be synthesised (by
> cconv) without the user actually ever feeding an explicit 'closure'
> construct into the compiler. After all, 'closure' isn't really in the
> language, is it?

No, indeed, it's not in the language.  The way it usually shows up is
when a macro stuffs a *value* in there, e.g.

    `(,(get foo 'frobbing-function) ...)

The more correct alternative

    `(funcall ',(get foo 'frobbing-function) ...)

tends to suffer from false-positive warnings about quoted lambdas when
the `frobbing-function` was not byte-compiled and comes from a file that
still doesn't use lexical-binding.



        Stefan




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

end of thread, other threads:[~2020-07-28 16:08 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-07-26 20:41 Minor simplification in byte-opt.el Stefan Monnier
2020-07-27  9:38 ` Mattias Engdegård
2020-07-27 14:24   ` Stefan Monnier
2020-07-28 14:43     ` Mattias Engdegård
2020-07-28 14:53       ` Stefan Monnier
2020-07-28 15:18         ` Mattias Engdegård
2020-07-28 16:08           ` Stefan Monnier

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

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