bug#42597: 27.1; (+ -0.0) returns +0.0 when compiled

["Mattias Engdegård" <mattiase@acm.org>]

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

29 juli 2020 kl. 15.35 skrev Alan Mackenzie <acm@muc.de>:

> This is an example of what happens when ignorant people rule the roost.
> -0.0 and +0.0 are identically the same thing.  It should not take a
> degree in mathematics (which I have) to realise this.  When you put
> mathematical nonsense into <whatever thing is producing -0.0> you cannot
> help but get nonsense back out.

Thanks Alan -- there are good arguments both for and against negative zero and I would happily discuss them over a little glass of something once it is possible to meet in person again. Now we don't have much choice since IEEE-754 is what it is and we should have very strong reasons for making changes that conflict with that standard.

At the very least we should be consistent. The effort is small enough (first patch below, I went with the (* x 1) variant).

The code did contain a fair amount of obsolete and/or incorrect comments and decisions, some relating to bug#1334 which is no longer relevant (Emacs didn't have bignums at the time). Today, the N-arg semantics of +, - (except for N=1), *, min and max (but notably not /) are equivalent to the corresponding left-folds of binary operations, which helps a lot. The second patch cleans up and improves optimisation for arithmetic operations generally.


[-- Attachment #2: 0001-Fix-byte-compilation-of-0.0-bug-42597.patch --]
[-- Type: application/octet-stream, Size: 2414 bytes --]

From 793e9252e3e946dd0d70cb5d461f7a29244b8811 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Mon, 3 Aug 2020 15:29:41 +0200
Subject: [PATCH 1/2] Fix byte-compilation of (+ -0.0) (bug#42597)

* lisp/emacs-lisp/bytecomp.el (byte-compile-associative):
Translate numerical identity expressions, such as (+ x) and (* x),
into (* x 1) since the previous translation (+ x 0) gets it wrong
for x = -0.0.
* test/lisp/emacs-lisp/bytecomp-tests.el
(byte-opt-testsuite-arith-data): Add test cases.
---
 lisp/emacs-lisp/bytecomp.el            | 6 +++---
 test/lisp/emacs-lisp/bytecomp-tests.el | 5 +++++
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 22e648e44b..8f76a3abb9 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -3733,7 +3733,7 @@ byte-compile-get-closed-var
 ;; Compile a function that accepts one or more args and is right-associative.
 ;; We do it by left-associativity so that the operations
 ;; are done in the same order as in interpreted code.
-;; We treat the one-arg case, as in (+ x), like (+ x 0).
+;; We treat the one-arg case, as in (+ x), like (* x 1).
 ;; in order to convert markers to numbers, and trigger expected errors.
 (defun byte-compile-associative (form)
   (if (cdr form)
@@ -3748,8 +3748,8 @@ byte-compile-associative
 	  (setq args (copy-sequence (cdr form)))
 	  (byte-compile-form (car args))
 	  (setq args (cdr args))
-	  (or args (setq args '(0)
-			 opcode (get '+ 'byte-opcode)))
+	  (or args (setq args '(1)
+			 opcode (get '* 'byte-opcode)))
 	  (dolist (arg args)
 	    (byte-compile-form arg)
 	    (byte-compile-out opcode 0))))
diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el
index c235dd43fc..894914300a 100644
--- a/test/lisp/emacs-lisp/bytecomp-tests.el
+++ b/test/lisp/emacs-lisp/bytecomp-tests.el
@@ -47,6 +47,11 @@ byte-opt-testsuite-arith-data
     (let ((a 1.0))				   (/ 3 a 2))
     (let ((a most-positive-fixnum) (b 2.0))	   (* a 2 b))
     (let ((a 3) (b 2))				   (/ a b 1.0))
+    (let ((a -0.0)) (+ a))
+    (let ((a -0.0)) (- a))
+    (let ((a -0.0)) (* a))
+    (let ((a -0.0)) (min a))
+    (let ((a -0.0)) (max a))
     (/ 3 -1)
     (+ 4 3 2 1)
     (+ 4 3 2.0 1)
-- 
2.21.1 (Apple Git-122.3)


[-- Attachment #3: 0002-Clean-up-and-improve-compilation-of-arithmetic-opera.patch --]
[-- Type: application/octet-stream, Size: 8122 bytes --]

From 8c1a405b2a39b03d167a744f63ac4c5504b9e9ad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Mon, 3 Aug 2020 16:29:06 +0200
Subject: [PATCH 2/2] Clean up and improve compilation of arithmetic operations

(Bug#42597)

* lisp/emacs-lisp/byte-opt.el (byte-optimize-associative-math)
(byte-optimize-min-max): Transform 3-arg min/max call to two 2-arg
calls, which is faster.
* lisp/emacs-lisp/bytecomp.el (byte-compile-associative): Rename to...
(byte-compile-variadic-numeric): ...this function and simplify,
fixing incorrect comments.  The 3-arg strength reduction is now
always done in the optimisers and is no longer needed here.
(byte-compile-min-max): New function.
(byte-compile-minus): Simplify, remove incorrect comment, and use
byte-compile-variadic-numeric.
(byte-compile-quo): Simplify and fix comment.
---
 lisp/emacs-lisp/byte-opt.el | 29 ++++++++----
 lisp/emacs-lisp/bytecomp.el | 93 +++++++++++++++++--------------------
 2 files changed, 62 insertions(+), 60 deletions(-)

diff --git a/lisp/emacs-lisp/byte-opt.el b/lisp/emacs-lisp/byte-opt.el
index 0d9c449b3b..4987596bf9 100644
--- a/lisp/emacs-lisp/byte-opt.el
+++ b/lisp/emacs-lisp/byte-opt.el
@@ -648,14 +648,23 @@ byte-optimize-associative-math
 	  (setq args (cons (car rest) args)))
       (setq rest (cdr rest)))
     (if (cdr constants)
-	(if args
-	    (list (car form)
-		  (apply (car form) constants)
-		  (if (cdr args)
-		      (cons (car form) (nreverse args))
-		      (car args)))
-	    (apply (car form) constants))
-	form)))
+        (let ((const (apply (car form) (nreverse constants))))
+	  (if args
+	      (append (list (car form) const)
+                      (nreverse args))
+	    const))
+      form)))
+
+(defun byte-optimize-min-max (form)
+  "Optimize `min' and `max'."
+  (let ((opt (byte-optimize-associative-math form)))
+    (if (and (consp opt) (memq (car opt) '(min max))
+             (= (length opt) 4))
+        ;; (OP x y z) -> (OP (OP x y) z), in order to use binary byte ops.
+        (list (car opt)
+              (list (car opt) (nth 1 opt) (nth 2 opt))
+              (nth 3 opt))
+      opt)))
 
 ;; Use OP to reduce any leading prefix of constant numbers in the list
 ;; (cons ACCUM ARGS) down to a single number, and return the
@@ -878,8 +887,8 @@ byte-optimize-concat
 (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 'max 'byte-optimizer #'byte-optimize-min-max)
+(put 'min 'byte-optimizer #'byte-optimize-min-max)
 
 (put '=   'byte-optimizer #'byte-optimize-binary-predicate)
 (put 'eq  'byte-optimizer #'byte-optimize-binary-predicate)
diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 8f76a3abb9..7ae8749ab4 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -3580,10 +3580,10 @@ narrow-to-region
 (byte-defop-compiler (% byte-rem)	2)
 (byte-defop-compiler aset		3)
 
-(byte-defop-compiler max		byte-compile-associative)
-(byte-defop-compiler min		byte-compile-associative)
-(byte-defop-compiler (+ byte-plus)	byte-compile-associative)
-(byte-defop-compiler (* byte-mult)	byte-compile-associative)
+(byte-defop-compiler max		byte-compile-min-max)
+(byte-defop-compiler min		byte-compile-min-max)
+(byte-defop-compiler (+ byte-plus)	byte-compile-variadic-numeric)
+(byte-defop-compiler (* byte-mult)	byte-compile-variadic-numeric)
 
 ;;####(byte-defop-compiler move-to-column	1)
 (byte-defop-compiler-1 interactive byte-compile-noop)
@@ -3730,30 +3730,36 @@ byte-compile-get-closed-var
   (if byte-compile--for-effect (setq byte-compile--for-effect nil)
     (byte-compile-out 'byte-constant (nth 1 form))))
 
-;; Compile a function that accepts one or more args and is right-associative.
-;; We do it by left-associativity so that the operations
-;; are done in the same order as in interpreted code.
-;; We treat the one-arg case, as in (+ x), like (* x 1).
-;; in order to convert markers to numbers, and trigger expected errors.
-(defun byte-compile-associative (form)
+;; Compile a pure function that accepts zero or more numeric arguments
+;; and has an opcode for the binary case.
+;; Single-argument calls are assumed to be numeric identity and are
+;; compiled as (* x 1) in order to convert markers to numbers and
+;; trigger type errors.
+(defun byte-compile-variadic-numeric (form)
+  (pcase (length form)
+    (1
+     ;; No args: use the identity value for the operation.
+     (byte-compile-constant (eval form)))
+    (2
+     ;; One arg: compile (OP x) as (* x 1). This is identity for
+     ;; all numerical values including -0.0, infinities and NaNs.
+     (byte-compile-form (nth 1 form))
+     (byte-compile-constant 1)
+     (byte-compile-out (get '* 'byte-opcode) 0))
+    (3
+     (byte-compile-form (nth 1 form))
+     (byte-compile-form (nth 2 form))
+     (byte-compile-out (get (car form) 'byte-opcode) 0))
+    (_
+     ;; >2 args: compile as a single function call.
+     (byte-compile-normal-call form))))
+
+(defun byte-compile-min-max (form)
+  "Byte-compile calls to `min' or `max'."
   (if (cdr form)
-      (let ((opcode (get (car form) 'byte-opcode))
-	    args)
-	(if (and (< 3 (length form))
-		 (memq opcode (list (get '+ 'byte-opcode)
-				    (get '* 'byte-opcode))))
-	    ;; Don't use binary operations for > 2 operands, as that
-	    ;; may cause overflow/truncation in float operations.
-	    (byte-compile-normal-call form)
-	  (setq args (copy-sequence (cdr form)))
-	  (byte-compile-form (car args))
-	  (setq args (cdr args))
-	  (or args (setq args '(1)
-			 opcode (get '* 'byte-opcode)))
-	  (dolist (arg args)
-	    (byte-compile-form arg)
-	    (byte-compile-out opcode 0))))
-    (byte-compile-constant (eval form))))
+      (byte-compile-variadic-numeric form)
+    ;; No args: warn and emit code that raises an error when executed.
+    (byte-compile-normal-call form)))
 
 \f
 ;; more complicated compiler macros
@@ -3768,7 +3774,7 @@ fset
 (byte-defop-compiler indent-to)
 (byte-defop-compiler insert)
 (byte-defop-compiler-1 function byte-compile-function-form)
-(byte-defop-compiler-1 - byte-compile-minus)
+(byte-defop-compiler (- byte-diff) byte-compile-minus)
 (byte-defop-compiler (/ byte-quo) byte-compile-quo)
 (byte-defop-compiler nconc)
 
@@ -3835,30 +3841,17 @@ byte-compile-concat
 	  ((byte-compile-normal-call form)))))
 
 (defun byte-compile-minus (form)
-  (let ((len (length form)))
-    (cond
-     ((= 1 len) (byte-compile-constant 0))
-     ((= 2 len)
-      (byte-compile-form (cadr form))
-      (byte-compile-out 'byte-negate 0))
-     ((= 3 len)
-      (byte-compile-form (nth 1 form))
-      (byte-compile-form (nth 2 form))
-      (byte-compile-out 'byte-diff 0))
-     ;; Don't use binary operations for > 2 operands, as that may
-     ;; cause overflow/truncation in float operations.
-     (t (byte-compile-normal-call form)))))
+  (if (/= (length form) 2)
+      (byte-compile-variadic-numeric form)
+    (byte-compile-form (cadr form))
+    (byte-compile-out 'byte-negate 0)))
 
 (defun byte-compile-quo (form)
-  (let ((len (length form)))
-    (cond ((< len 2)
-	   (byte-compile-subr-wrong-args form "1 or more"))
-	  ((= len 3)
-	   (byte-compile-two-args form))
-	  (t
-	   ;; Don't use binary operations for > 2 operands, as that
-	   ;; may cause overflow/truncation in float operations.
-	   (byte-compile-normal-call form)))))
+  (if (= (length form) 3)
+      (byte-compile-two-args form)
+    ;; N-ary `/' is not the left-reduction of binary `/' because if any
+    ;; argument is a float, then everything is done in floating-point.
+    (byte-compile-normal-call form)))
 
 (defun byte-compile-nconc (form)
   (let ((len (length form)))
-- 
2.21.1 (Apple Git-122.3)