From: Paul Eggert <eggert@cs.ucla.edu>
To: Eli Zaretskii <eliz@gnu.org>
Cc: emacs-devel@gnu.org
Subject: Re: Strange problems with data-tests.el
Date: Sun, 12 Aug 2018 15:43:14 -0700 [thread overview]
Message-ID: <486fc1b9-88a7-93e0-dd45-4f8f1d8a77d1@cs.ucla.edu> (raw)
In-Reply-To: <831sb3l9cx.fsf@gnu.org>
[-- Attachment #1: Type: text/plain, Size: 1004 bytes --]
Eli Zaretskii wrote:
> This all looks like a compiler bug
Yes and no. In this area, GCC deliberately doesn't conform to the C standard
unless you use an option like -fexcess-precision=standard that causes GCC to
conform but often generate worse code.
The odd results you observed can occur when a C 'double' variable spontaneously
changes value, e.g., when an 80-bit floating-point register is spilled to a
64-bit memory location. I reproduced the problem on Fedora 28 and fixed it by
installing the attached patch into master; please give it a try.
I suspect that it's only bad luck that exposed the problem, that is, that the
recent bignum or misc changes triggered the bug only because they caused GCC to
optimize in a different way. In theory if we're unlucky the bug could recur,
since GCC doesn't make promises in this area. However, I changed arithcompare in
such a way that GCC would have to be pretty perverse to cause the problem to
resurface, and similarly for other C compilers.
[-- Attachment #2: 0001-Port-better-to-x86-fexcess-precision-fast.patch --]
[-- Type: text/x-patch, Size: 3508 bytes --]
From a84cef90957f2379cc0df6bd908317fc441971ce Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 12 Aug 2018 15:28:20 -0700
Subject: [PATCH] Port better to x86 -fexcess-precision=fast
Problem reported by Eli Zaretskii in:
https://lists.gnu.org/r/emacs-devel/2018-08/msg00380.html
* src/data.c (arithcompare): Work around incompatibility
between gcc -fexcess-precision=fast and the C standard on x86,
by capturing the results of floating-point comparisons before
the excess precision spontaneously decays. Although this fix
might not work in general, it does work here and is probably
good enough for the platforms we care about.
---
src/data.c | 43 +++++++++++++++++++++++++++++--------------
1 file changed, 29 insertions(+), 14 deletions(-)
diff --git a/src/data.c b/src/data.c
index c8a9c6b378..7b8dd45c94 100644
--- a/src/data.c
+++ b/src/data.c
@@ -2492,7 +2492,7 @@ arithcompare (Lisp_Object num1, Lisp_Object num2,
{
double f1, f2;
EMACS_INT i1, i2;
- bool fneq;
+ bool lt, eq, gt;
bool test;
CHECK_NUMBER_COERCE_MARKER (num1);
@@ -2502,10 +2502,13 @@ arithcompare (Lisp_Object num1, Lisp_Object num2,
return bignumcompare (num1, num2, comparison);
/* If either arg is floating point, set F1 and F2 to the 'double'
- approximations of the two arguments, and set FNEQ if floating-point
- comparison reports that F1 is not equal to F2, possibly because F1
- or F2 is a NaN. Regardless, set I1 and I2 to integers that break
- ties if the floating-point comparison is either not done or reports
+ approximations of the two arguments, and set LT, EQ, and GT to
+ the <, ==, > floating-point comparisons of F1 and F2
+ respectively, taking care to avoid problems if either is a NaN,
+ and trying to avoid problems on platforms where variables (in
+ violation of the C standard) can contain excess precision.
+ Regardless, set I1 and I2 to integers that break ties if the
+ floating-point comparison is either not done or reports
equality. */
if (FLOATP (num1))
@@ -2528,7 +2531,9 @@ arithcompare (Lisp_Object num1, Lisp_Object num2,
to I2 will break the tie correctly. */
i1 = f2 = i2 = XFIXNUM (num2);
}
- fneq = f1 != f2;
+ lt = f1 < f2;
+ eq = f1 == f2;
+ gt = f1 > f2;
}
else
{
@@ -2539,39 +2544,49 @@ arithcompare (Lisp_Object num1, Lisp_Object num2,
converse of comparing float to integer (see above). */
i2 = f1 = i1;
f2 = XFLOAT_DATA (num2);
- fneq = f1 != f2;
+ lt = f1 < f2;
+ eq = f1 == f2;
+ gt = f1 > f2;
}
else
{
i2 = XFIXNUM (num2);
- fneq = false;
+ eq = true;
}
}
+ if (eq)
+ {
+ /* Break a floating-point tie by comparing the integers. */
+ lt = i1 < i2;
+ eq = i1 == i2;
+ gt = i1 > i2;
+ }
+
switch (comparison)
{
case ARITH_EQUAL:
- test = !fneq && i1 == i2;
+ test = eq;
break;
case ARITH_NOTEQUAL:
- test = fneq || i1 != i2;
+ test = !eq;
break;
case ARITH_LESS:
- test = fneq ? f1 < f2 : i1 < i2;
+ test = lt;
break;
case ARITH_LESS_OR_EQUAL:
- test = fneq ? f1 <= f2 : i1 <= i2;
+ test = lt | eq;
break;
case ARITH_GRTR:
- test = fneq ? f1 > f2 : i1 > i2;
+ test = gt;
break;
case ARITH_GRTR_OR_EQUAL:
- test = fneq ? f1 >= f2 : i1 >= i2;
+ test = gt | eq;
break;
default:
--
2.17.1
next prev parent reply other threads:[~2018-08-12 22:43 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-12 18:37 Strange problems with data-tests.el Eli Zaretskii
2018-08-12 22:43 ` Paul Eggert [this message]
2018-08-13 15:05 ` Eli Zaretskii
2018-08-13 16:24 ` Paul Eggert
2018-08-13 16:49 ` Eli Zaretskii
2018-08-13 17:02 ` Paul Eggert
2018-08-13 10:06 ` Pip Cet
2018-08-13 14:44 ` Eli Zaretskii
2018-08-13 14:58 ` Pip Cet
2018-08-13 16:43 ` Paul Eggert
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=486fc1b9-88a7-93e0-dd45-4f8f1d8a77d1@cs.ucla.edu \
--to=eggert@cs.ucla.edu \
--cc=eliz@gnu.org \
--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.