Received: (at 49659) by debbugs.gnu.org; 20 Jul 2021 20:52:10 +0000
From debbugs-submit-bounces@debbugs.gnu.org Tue Jul 20 16:52:10 2021
Received: from localhost ([127.0.0.1]:35514 helo=debbugs.gnu.org)
by debbugs.gnu.org with esmtp (Exim 4.84_2)
(envelope-from )
id 1m5win-0002aL-R1
for submit@debbugs.gnu.org; Tue, 20 Jul 2021 16:52:10 -0400
Received: from eggs.gnu.org ([209.51.188.92]:44228)
by debbugs.gnu.org with esmtp (Exim 4.84_2)
(envelope-from ) id 1m5wil-0002Zu-8R
for 49659@debbugs.gnu.org; Tue, 20 Jul 2021 16:52:08 -0400
Received: from fencepost.gnu.org ([2001:470:142:3::e]:58508)
by eggs.gnu.org with esmtp (Exim 4.90_1)
(envelope-from )
id 1m5wif-0003PS-35; Tue, 20 Jul 2021 16:52:01 -0400
Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=45238 helo=ribbon)
by fencepost.gnu.org with esmtpsa
(TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1)
(envelope-from )
id 1m5wie-0006Um-S0; Tue, 20 Jul 2021 16:52:01 -0400
From: =?utf-8?Q?Ludovic_Court=C3=A8s?=
To: Maxime Devos
Subject: Re: bug#49659: [PATCH core-updates] gnu: guile: Fix failing tests
on i686-linux.
References: <20210720112712.25905-1-maximedevos@telenet.be>
<871r7tks2i.fsf@gnu.org>
Date: Tue, 20 Jul 2021 22:51:59 +0200
In-Reply-To:
(Maxime Devos's message of "Tue, 20 Jul 2021 18:55:52 +0200")
Message-ID: <87fsw8k8sw.fsf_-_@gnu.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
X-Spam-Score: -2.3 (--)
X-Debbugs-Envelope-To: 49659
Cc: 49659@debbugs.gnu.org
X-BeenThere: debbugs-submit@debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id:
List-Unsubscribe: ,
List-Archive:
List-Post:
List-Help:
List-Subscribe: ,
Errors-To: debbugs-submit-bounces@debbugs.gnu.org
Sender: "Debbugs-submit"
X-Spam-Score: -3.3 (---)
Maxime Devos skribis:
> Ludovic Court=C3=A8s schreef op di 20-07-2021 om 15:55 [+0200]:
[...]
>> 1. Should we make it conditional on
>> (or (string-prefix? "i686-" %host-type)
>> (string-prefix? "i586-" %host-type))
>
> Rather, (target-x86-32?). target-x86-32? also recognises "i486-linux-gnu"
> even though that's not a =E2=80=98supported=E2=80=99 cross-target.
Yes, makes sense.
>> ? (I wonder why armhf-linux doesn=E2=80=99t have the same problem.)
>
> AFAIK floats & doubles on arm don't have excess precision.
>
> Floating-point numbers are either 32-bit or 64-bit,
> unlike in x86, where the floating-point registers are 80-bit
> but (sizeof) double=3D=3D8 (64 bits).
>
> (I'm ignoring MMX, SSE and the like.)
>
> I don't know any architectures beside x86 which have excess precision.
> "-fexcess-precision=3Dstandard" should be harmless on architectures
> that don't have excess precision.
>
> I'd make it unconditional, but conditional on x86-target? should work
> for all =E2=80=98supported=E2=80=99 targets in Guix.
Alright.
I=E2=80=99d still err on the side of making the change only for target-x86-=
32?,
because that=E2=80=99s the only case where we know it=E2=80=99s needed.
>> 2. Is there any downside to compiling all of libguile with this flag?
>
> I searched with "git grep -F double" and "git grep -F float".
> Floating-point arithmetic doen't seem to be used much outside numbers.c.
>
> There's vm-engine.c, but the results of the calculations are written
> to some (stack?) memory (not a register), so the excess precision
> would be thrown away anyway, so I don't expect a slow-down.
>
> No code appears to be relying on excess precision.
OK.
>> 3. Do we have a clear explanation of why =E2=80=98-fexcess-precision=
=3Dfast=E2=80=99
>> (the default) would lead to failures in =E2=80=98numbers.test=E2=80=
=99?
>
> The problem I think is that the rounding choices made in
> scm_i_inexact_floor_divide
> must be consistent with those made in
> scm_i_inexact_floor_quotient
> and=20
> scm_i_inexact_floor_remainder
> (There are tests testing whether the results agree.)
>
> "-fexcess-precision=3Dstandard" reduces the degrees of freedom GCC has
> in choosing when to round, so it is more likely the rounding choices
> coincide? It's only an untested hypothesis though.
>
> FWIW, I think this line:
>
> /* in scm_i_inexact_floor_remainder */
> return scm_i_from_double (x - y * floor (x / y));
>
> should be (for less fragility in case GCC changes its optimisations and
> register allocation / spilling)
>
> /* in scm_i_inexact_floor_remainder */
> return scm_i_from_double (x - y * (double) floor (x / y));
>
> even then, there's no guarantee the rounding choices for x/y
> are the same in scm_i_inexact_floor_divide, scm_i_inexact_floor_remainder
> and scm_i_inexact_floor_quotient.
Makes sense. Seems to me that this should simply be implemented
differently to avoid the inconsistency in the first place (or one could
ignore IA32 altogether=E2=80=A6).
> I dunno if 'floor' is broken. Let me explain why this output is possible=
for a
> well-implemented 'floor':
>
> I want to note that printf("%f", floor(x/y))
> can display 16 different strings:
>
> GCC can choose to round 'x' and/or 'y' by moving it from a register to =
stack memory.
> (doesn't apply here I think because SCM values discard the excess preci=
sion)
>
> GCC can choose to round the result of x/y (same reasons)
>
> GCC can choose to round the result of floor(x/y) (same reasons)
>
> Likewise, printf("%f", x/y) can display 8 different strings, and the roun=
ding
> choices may be different from those made for printf("%f", floor(x/y)).
>
> So this inconsistency (x/y=3D91.00... > 90.00 =3D floor(x/y)) doesn't re=
ally
> surprise me. A more concrete scenario: suppose the CPU is configured to =
round
> upwards, and 'floor' is a mapping between extended-precision floating-poi=
nt numbers.
>
> Let 'x' and 'y' be two floating-point numbers, such that:
>
> (1) the mathematical value of 'x/y' is slightly less than 91
> (2) 'x/y' when calculated in extended precision is slightly less than 91.
> Denote this approximation as F1.
> (3) 'x/y' when calculated in double precision is 91 (or slightly larger)
> (due to the =E2=80=98rounding upwards=E2=80=99 mode, in =E2=80=98rou=
nding downwards=E2=80=99 it might
> have been slightly less than 91 as in (2))
> Denote this approximation as F2.
>
> Then [floor(x/y)=3D] floor(F1)=3Dfloor(90.999...)=3D90.0,
> and [x/y=3D] F2=3D91.0, thus we seemingly have x/y >=3D 1 + floor(x/y),
> even though that's mathematically nonsense.
>
> Thus, by choosing when to round (in-)appropriately, it is possible (on x8=
6)
> that printf("x/y=3D%f, floor(x/y)=3D%f",x/y,floor(x/y)) will output "x/y=
=3D91.0 floor(x/y)=3D90.0".
I=E2=80=99m no expert but that makes sense to me.
Could you send an updated patch?
If you think of a way to fix the issue in Guile itself, we can also do
that. :-)
Thanks for the investigation & explanation!
Ludo=E2=80=99.