Re: Guile 64-bit Windows support, redux

Michael Käppler via "Developers list for Guile, the GNU extensibility library" <[email protected]> Thu, 27 Feb 2025 21:52:05 +0100
Newsgroups gmane.lisp.guile.devel
Message-ID <[email protected]>

Am 27.02.2025 um 17:55 schrieb Mike Gran:
>> Can you share details about your compilation and testing setup?
> I applied the patches to main and added some VMs at
> github.com/spk121/guile-jonas
Thanks, very helpful.
>> Also, what exactly is not functional in your build?
> Well, the very first problems I run into have nothing to do with the
> 64-bit problem, but, just basic bitrot.  For some time, I had tried to ensure
> that the 32-bit, non-threaded, non-LTO, non-JIT version of Guile
> on MinGW kept working. But, it is broken on `main` again. The very first
> problems are
> - need win32 version of pagesize()
> - need to make sure that dlopen searches PATH on Win32 and handles
>    DLLs of the form libfoo-X.dll in load-foreign-library
>
> I've got patches for these.
Okay, so it seems these problems are somewhat orthogonal to the ones we
address
with our patches.
>
>> I have some more patches here that I contributed to fix Guile-Windows issues
>> in Lilypond. I can try to set up a repo on Github with all these patches applied
>> on top of current Guile 'main'.
>> However, as we currently use Guile 3.0.10 to do releases, I would first have to
>> make sure it still works with main.
> Don't worry about rebasing for now. I'd just like to see what you have
> working already in whatever form it is currently. My hope is that we can
> prefer your patches, and then I can tweak around them for any problems
> that remain.
Find the patches attached. I'm hoping to get a Mingw64 build environment
running under Windows 10 going with
the setup described in your repo.

Michael
0009-mpz-integer-do-not-convert-to-bignum-if-inum-is-suff.patch (text/plain, 1.3 KB)
From d637d8155566087243ee4afbbcd4296ad6f2480f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20K=C3=A4ppler?= <[email protected]>
Date: Tue, 25 Jun 2024 23:30:41 +0200
Subject: [PATCH 3/3] mpz->integer: do not convert to bignum if inum is
 sufficient

Before, `take_mpz` would convert to bignum if the `mpz` value
does not fit into a signed long, even though `scm_t_inum` is not
coupled to `long` anymore.  This leads to problems with functions
that expect values that numerically fit into a fixnum to be of
inum type, e.g., during bytecode compilation of long-long-immediates,
where `ash` returns a bignum, while the assembler requires an inum.

Call `scm_integer_from_mpz` instead, which will convert back to inum
if the number fits into an inum.

* libguile/integers.c (take_mpz): use `scm_integer_from_mpz`
---
 libguile/integers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libguile/integers.c b/libguile/integers.c
index 1560c5d17..092345b1d 100644
--- a/libguile/integers.c
+++ b/libguile/integers.c
@@ -335,7 +335,7 @@ take_mpz (mpz_ptr mpz)
   if (mpz_fits_slong_p (mpz))
     ret = scm_from_inum (mpz_get_si (mpz));
   else
-    ret = scm_from_bignum (make_bignum_from_mpz (mpz));
+    ret = scm_integer_from_mpz (mpz);
   mpz_clear (mpz);
   return ret;
 }
-- 
2.34.1
0008-Avoid-using-long-integers-literals.patch (text/plain, 2.1 KB)
From 7d21acce0f94b8fd372e6bb3e55d8b04d964c85b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20K=C3=A4ppler?= <[email protected]>
Date: Wed, 19 Jun 2024 22:07:09 +0200
Subject: [PATCH 2/3] Avoid using `long` integers literals

Bit manipulation on `long` integer literals caused overflow in
some cases (e.g., when calculating high powers of two) on Windows,
where `long` is only 32 bit width.

* libguile/integers.c (scm_integer_expt_ii, scm_integer_round_rsh_iu,
  scm_integer_bit_extract_i): Use `scm_t_inum` type instead of `long`
  in places where `long` could overflow.
---
 libguile/integers.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/libguile/integers.c b/libguile/integers.c
index aab49428e..1560c5d17 100644
--- a/libguile/integers.c
+++ b/libguile/integers.c
@@ -2299,7 +2299,7 @@ scm_integer_expt_ii (scm_t_inum n, scm_t_inum k)
   if (n == 2)
     {
       if (k < SCM_I_FIXNUM_BIT - 1)
-        return SCM_I_MAKINUM (1L << k);
+        return SCM_I_MAKINUM (((scm_t_inum) 1) << k);
       if (k < 64)
         return scm_integer_from_uint64 (((uint64_t) 1) << k);
       size_t nlimbs = k / (sizeof (mp_limb_t)*8) + 1;
@@ -2458,12 +2458,12 @@ scm_integer_round_rsh_iu (scm_t_inum n, unsigned long count)
     {
       scm_t_inum q = SCM_SRS (n, count);
 
-      if (0 == (n & (1L << (count-1))))
+      if (0 == (n & (((scm_t_inum) 1) << (count-1))))
         return SCM_I_MAKINUM (q);                /* round down */
-      else if (n & ((1L << (count-1)) - 1))
+      else if (n & ((((scm_t_inum) 1) << (count-1)) - 1))
         return SCM_I_MAKINUM (q + 1);            /* round up */
       else
-        return SCM_I_MAKINUM ((~1L) & (q + 1));  /* round to even */
+        return SCM_I_MAKINUM (~((scm_t_inum) 1) & (q + 1));  /* round to even */
     }
 }
 
@@ -2505,7 +2505,7 @@ scm_integer_bit_extract_i (scm_t_inum n, unsigned long start,
 
   /* mask down to requisite bits */
   bits = MIN (bits, SCM_I_FIXNUM_BIT);
-  return SCM_I_MAKINUM (n & ((1L << bits) - 1));
+  return SCM_I_MAKINUM (n & ((((scm_t_inum) 1) << bits) - 1));
 }
 
 SCM
-- 
2.34.1
0007-Fix-inum-negation.patch (text/plain, 2.2 KB)
From 6f1ff1036bec220c0ade16d0ae37a82293a13c35 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20K=C3=A4ppler?= <[email protected]>
Date: Sat, 15 Jun 2024 22:18:56 +0200
Subject: [PATCH 1/3] Fix inum negation

The use of `negative_long` can lead to overflow if called
on inums that do not fit into a `long`.  This led to
problems when normalizing bignums on Windows.

* libguile/integers.c: use new `negative_inum` for negating inums,
  drop now unused function `negative_long`.
---
 libguile/integers.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/libguile/integers.c b/libguile/integers.c
index 380ff193c..aab49428e 100644
--- a/libguile/integers.c
+++ b/libguile/integers.c
@@ -109,13 +109,6 @@ long_magnitude (long l)
   return l < 0 ? ~mag + 1 : mag;
 }
 
-static inline long
-negative_long (unsigned long mag)
-{
-  ASSERT (mag <= (unsigned long) LONG_MIN);
-  return ~mag + 1;
-}
-
 static inline int64_t
 negative_int64 (uint64_t mag)
 {
@@ -141,6 +134,14 @@ inum_magnitude (scm_t_inum i)
   return mag;
 }
 
+static inline scm_t_inum
+negative_inum (scm_t_bits mag)
+{
+  scm_t_inum i = ~mag + 1;
+  ASSERT (i >= SCM_MOST_NEGATIVE_FIXNUM);
+  return i;
+}
+
 static struct scm_bignum *
 allocate_bignum (size_t nlimbs)
 {
@@ -313,7 +314,7 @@ normalize_bignum (struct scm_bignum *z)
     {
     case -1:
       if (bignum_limbs (z)[0] <= inum_magnitude (SCM_MOST_NEGATIVE_FIXNUM))
-        return SCM_I_MAKINUM (negative_long (bignum_limbs (z)[0]));
+        return SCM_I_MAKINUM (negative_inum (bignum_limbs (z)[0]));
       break;
     case 0:
       return SCM_INUM0;
@@ -429,7 +430,7 @@ negative_uint32_to_int32 (uint32_t magnitude, int32_t *val)
 {
   if (magnitude > long_magnitude (INT32_MIN))
     return 0;
-  *val = negative_long (magnitude);
+  *val = -magnitude;
   return 1;
 }
 
@@ -3070,7 +3071,7 @@ scm_integer_mul_ii (scm_t_inum x, scm_t_inum y)
       if (negative)
         {
           if (lo <= inum_magnitude (SCM_MOST_NEGATIVE_FIXNUM))
-            return SCM_I_MAKINUM (negative_long (lo));
+            return SCM_I_MAKINUM (negative_inum (lo));
         }
       else if (lo <= SCM_MOST_POSITIVE_FIXNUM)
         return SCM_I_MAKINUM (lo);
-- 
2.34.1