* Guile 64-bit Windows support, redux [not found] <1629803116.370682.1686084646758.ref@mail.yahoo.com> @ 2023-06-06 20:50 ` Mike Gran 2023-06-06 20:56 ` Jean Abou Samra ` (3 more replies) 0 siblings, 4 replies; 23+ messages in thread From: Mike Gran @ 2023-06-06 20:50 UTC (permalink / raw) To: guile-devel@gnu.org Hello Guile, There have been a few times where I made Guile work on MinGW -- usually the 32-bit, unthreaded variety -- because I wanted it for my own entertainment. Often around the time of the Lisp Game Jam. The Game Jam just happened, so I was poking it again. Although I never actually delivered my game this year. Janneke has also poked at Windows support a few times as well, and he actually got it to run on 64-bit Windows. I'd be willing [1] to update the patches for MinGW support for review and inclusion into the main branch for 3.0.10 or 3.2 but, only if maintainers are open, in principle, to committing the most disruptive patch: the one where all long integers become intptr_t to deal with Window's stupid 4-byte longs on 64-bit OSs. Without agreeing on that as a possibility, 64-bit Windows support is DOA. There are a couple of slightly outdated versions of janekke's 64-bit patch. Here, for example: https://git.savannah.gnu.org/cgit/guile.git/commit/?h=wip-mingw&id=76950b4281c7dfff78b9ead6d3d62c070bbc1f13 Lately, with GitHub providing decent zero-cost Windows images for their CI/CD actions, it should be possible to do a nightly pull and 'make distcheck' off the main branch for Windows, Ubuntu, and MacOS, without requiring the developer to use non-free tools directly. (But lots of non-free Javascript and who-knows-what on GitHub's side of things.) If this could be set up, it might stop the Windows build from getting stale so quickly. Also, it might be that this isn't worth doing. After all, you can run Guile on Cygwin and Guix on WSL on Windows 10/11 already. But some projects that depend on Guile do deliver on Windows using customized versions of 2.2 or 1.8. Let me know what you think. -Mike Gran [1] Last two times I volunteered myself for something on Guile/Guix, I got quite ill. Let's not hope for a repeat. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Guile 64-bit Windows support, redux 2023-06-06 20:50 ` Guile 64-bit Windows support, redux Mike Gran @ 2023-06-06 20:56 ` Jean Abou Samra 2023-06-06 21:10 ` [EXT] " Thompson, David ` (2 subsequent siblings) 3 siblings, 0 replies; 23+ messages in thread From: Jean Abou Samra @ 2023-06-06 20:56 UTC (permalink / raw) To: Mike Gran, guile-devel@gnu.org [-- Attachment #1.1: Type: text/plain, Size: 763 bytes --] Le mardi 06 juin 2023 à 20:50 +0000, Mike Gran a écrit : > Also, it might be that this isn't worth doing. After all, you can run > Guile on Cygwin and Guix on WSL on Windows 10/11 already. > But some projects that depend on Guile do deliver on > Windows using customized versions of 2.2 or 1.8. I would like to stress the importance of Windows support in Guile, and I thank you for working on it. In spite of LilyPond being compatible with Guile 3 for more than a year, we are still on Guile 2.2 solely because of lack of Windows support in Guile 3. LilyPond is not primarily targeted at programmers but musicians, so a large part of our user base is on Windows and it is absolutely unthinkable for us to drop our Windows binaries. Jean [-- Attachment #1.2: Type: text/html, Size: 1335 bytes --] [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* [EXT] Guile 64-bit Windows support, redux 2023-06-06 20:50 ` Guile 64-bit Windows support, redux Mike Gran 2023-06-06 20:56 ` Jean Abou Samra @ 2023-06-06 21:10 ` Thompson, David 2023-06-06 21:58 ` Mike Gran 2023-06-08 19:50 ` Maxime Devos 2023-10-29 21:34 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 3 siblings, 1 reply; 23+ messages in thread From: Thompson, David @ 2023-06-06 21:10 UTC (permalink / raw) To: Mike Gran; +Cc: guile-devel@gnu.org Hi Mike, On Tue, Jun 6, 2023 at 4:51 PM Mike Gran <spk121@yahoo.com> wrote: > > Hello Guile, > > There have been a few times where I made Guile work on > MinGW -- usually the 32-bit, unthreaded variety -- because I wanted it > for my own entertainment. Often around the time of the Lisp Game Jam. > The Game Jam just happened, so I was poking it again. Although > I never actually delivered my game this year. Game jam time is also when I lament the lack of real Windows support. I'm very interested in being able to provide Windows releases of games built with guile-sdl2 and/or chickadee. The Common Lisp and Fennel folks have no problem shipping things for Windows users. > Janneke has also poked at Windows support a few times as well, and > he actually got it to run on 64-bit Windows. Did JIT work? When I tried this years ago it didn't. Would be a major victory if it works now. > I'd be willing [1] to update the patches for MinGW support for > review and inclusion into the main branch > for 3.0.10 or 3.2 but, only if maintainers are open, in principle, > to committing the most disruptive patch: the one where all > long integers become intptr_t to deal with Window's stupid > 4-byte longs on 64-bit OSs. Without agreeing on that as a > possibility, 64-bit Windows support is DOA. > > There are a couple of slightly outdated versions of janekke's 64-bit patch. > Here, for example: > > https://git.savannah.gnu.org/cgit/guile.git/commit/?h=wip-mingw&id=76950b4281c7dfff78b9ead6d3d62c070bbc1f13 > > Lately, with GitHub providing decent zero-cost Windows images > for their CI/CD actions, it should be possible to do a nightly pull > and 'make distcheck' off the main branch for Windows, Ubuntu, > and MacOS, without requiring the developer to use non-free tools > directly. (But lots of non-free Javascript and who-knows-what on > GitHub's side of things.) I'm obviously not a maintainer but I do want to be vocal in my support for a good Windows story. Despite how much I personally do not want to use Windows, I do want users who do to be able to run my software. > If this could be set up, it might stop the Windows build from getting > stale so quickly. > > Also, it might be that this isn't worth doing. After all, you can run > Guile on Cygwin and Guix on WSL on Windows 10/11 already. > But some projects that depend on Guile do deliver on > Windows using customized versions of 2.2 or 1.8. WSL is cool and all, but personally I want to ship native Windows stuff that "just works" as much as possible. I'm not sure what the graphics story is like on WSL these days, but in the past dealing with X11 compatibility and GPUs was not easy, or so I've heard. So for me, I'd really like to see the MinGW build work acceptably (threads, JIT, etc.) Building with MSVC would be even better but I'll take what I can get. :) I just get jealous whenever I look at some new language implementation (or just some other Scheme) and see that it can do native Windows, Linux, and MacOS builds. - Dave ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [EXT] Guile 64-bit Windows support, redux 2023-06-06 21:10 ` [EXT] " Thompson, David @ 2023-06-06 21:58 ` Mike Gran 0 siblings, 0 replies; 23+ messages in thread From: Mike Gran @ 2023-06-06 21:58 UTC (permalink / raw) To: Thompson, David; +Cc: guile-devel@gnu.org On Tuesday, June 6, 2023 at 02:10:25 PM PDT, Thompson, David <dthompson2@worcester.edu> wrote: ... >> Janneke has also poked at Windows support a few times as well, and >> he actually got it to run on 64-bit Windows. >Did JIT work? When I tried this years ago it didn't. Would be a major >victory if it works now. There were patches that made it compile, but, I don't think it ever properly worked. I always build without JIT. I haven't tried to build with JIT in a while. ... > I'm obviously not a maintainer but I do want to be vocal in my support > for a good Windows story. Despite how much I personally do not want > to use Windows, I do want users who do to be able to run my software. Same. ... > WSL is cool and all, but personally I want to ship native Windows > stuff that "just works" as much as possible. I'm not sure what the > graphics story is like on WSL these days, but in the past dealing with > X11 compatibility and GPUs was not easy, or so I've heard. So for me, > I'd really like to see the MinGW build work acceptably (threads, JIT, > etc.) Building with MSVC would be even better but I'll take what I > can get. :) I just get jealous whenever I look at some new language > implementation (or just some other Scheme) and see that it can do > native Windows, Linux, and MacOS builds. I have seen guile-gi launch a program on MinGW using MinGW's Gtk libraries. I have no benchmarks. I did attempt a build with MSVC (which is a nightmare because you have to run autoconf in MSYS and use a batch script to translate gcc flags into cl flags). It looked pretty bad: so many compilation errors. I think you'd have to fix all of gnulib and then pull all of gnulib into Guile, haha. -Mike ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Guile 64-bit Windows support, redux 2023-06-06 20:50 ` Guile 64-bit Windows support, redux Mike Gran 2023-06-06 20:56 ` Jean Abou Samra 2023-06-06 21:10 ` [EXT] " Thompson, David @ 2023-06-08 19:50 ` Maxime Devos 2023-06-08 20:46 ` Mike Gran 2023-10-29 21:34 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 3 siblings, 1 reply; 23+ messages in thread From: Maxime Devos @ 2023-06-08 19:50 UTC (permalink / raw) To: Mike Gran, guile-devel@gnu.org [-- Attachment #1.1.1: Type: text/plain, Size: 9604 bytes --] Op 06-06-2023 om 22:50 schreef Mike Gran: > Hello Guile, > > There have been a few times where I made Guile work on > MinGW -- usually the 32-bit, unthreaded variety -- because I wanted it > for my own entertainment. Often around the time of the Lisp Game Jam. > The Game Jam just happened, so I was poking it again. Although > I never actually delivered my game this year. > > Janneke has also poked at Windows support a few times as well, and > he actually got it to run on 64-bit Windows. > > I'd be willing [1] to update the patches for MinGW support for > review and inclusion into the main branch > for 3.0.10 or 3.2 but, only if maintainers are open, in principle, > to committing the most disruptive patch: the one where all > long integers become intptr_t to deal with Window's stupid > 4-byte longs on 64-bit OSs. Without agreeing on that as a > possibility, 64-bit Windows support is DOA. (The following response should be read with the following proverb in mind: <https://en.wiktionary.org/wiki/de_beste_stuurlui_staan_aan_wal> Proverb de beste stuurlui staan aan wal 1. It's easy to criticize other people's work when you're not the one doing it. Compare also a backseat driver. I haven't tried any alternative approached myself.) (The following response is a bit muddled -- it initially says I don't follow but later I will. Still, there is some stuff of which I really disagree with the implementation.) I don't quite follow why the change from 'long' to 'intptr_t'. I mean, reading the commit message you linked to: > As long is used for SCM_FIXNUM_BIT, that would mean incompatible > .go files, [...] ... how is this a problem? .go files are already incompatible between architectures and Guile already supports cross-compilation; adding incompatibility between OS doesn't really make a difference. (For other readers: SCM_FIXNUM_BIT has been renamed to SCM_I_FIXNUM_BIT.) Also: > [...] and a waste of cell space. IIUC, Guile currently uses a 'long' (minus a few bits for typing) embedded in a SCM/scm_t_bits/uintptr_t for fixnums. If sizeof(long)<sizeof(uintptr_t), that's indeed a waste (I assume that's what you are referring to). In that case, why not change the fixnum implementation to use 'uintptr_t' or 'scm_t_bits' instead of 'long'? Reading a bit further, such a thing appears to already have thought of: > So we would like to use long long, but the GMP interface > uses long. and the new problem is already resolved. Though I don't get why a new SCM_INTPTR_T_BIT is defined instead of changing SCM_FIXNUM_BIT to use (64-bit) uintptr_t instead of (32-bit) long -- I mean, wasn't compatibility and space-savings (i.e. avoiding bignums) a concern? Also, why not patch GMP to also have 'long long' variants? Sure, it will take a while to reach distributions ... but Windows doesn't really do distributions much anyway, so that doesn't seem a problem to me. That would avoid a lot of GMP-related changes and avoid the need for the non-standard minigmp on Windows. (There are some downsides, like having to pick different functions depending on whether 'long' or 'long long' is used, though to a degree that can be resolved with some #define. Maybe that was one of the reasons why not.) Looking at the changelog, I'm thinking it changes too much: changing configure.ac and {numbers,integers}.{c,h}, sure, but why why hash.c, symbols.c, array-map.c? It also changes too little: it forgets to change scm_t_inum from 'long' to 'uintptr_t' -- if you want to have uintptr_t (minus type bits) fixnums, then you need to change scm_t_inum, otherwise the comment becomes invalid: /* Immediate Numbers, also known as fixnums * * Inums are exact integers that fit within an SCM word * (along with two tagging bits). * * In the current implementation, Inums must also fit within a long * because that's what GMP's mpz_*_si functions accept. */ typedef long scm_t_inum; (It also becomes invalid because with the new minigmp, minigmp accepts uintptr_t instead of long.) Looking closer at some of them, the changes to array_compare makes sense to me, even if Windows didn't have 32-bit longs, but some of the other changes don't. For example, the first change is to scm_array_in_bounds_p. Now you can have INTPTR_T_MAX dimensions instead of LONG_MAX dimensions! Seems useless to me; 2**32 is already a stupidly high limit. The change to array_to_list makes sense to me, even if long were 64-bit. Now consider the change to scm_array_contents: > - SCM_I_ARRAY_BASE (ra) % SCM_LONG_BIT || > - len % SCM_LONG_BIT)) > + SCM_I_ARRAY_BASE (ra) % SCM_INTPTR_T_BIT || > + len % SCM_INTPTR_T_BIT)) Neither version makes much sense to me -- SCM_I_ARRAY_BASE nominally returns a size_t, not an intptr_t: #define SCM_I_ARRAY_BASE(a) ((size_t) SCM_CELL_WORD_2 (a)) so it should use SCM_SIZE_T_BIT instead of SCM_LONG_BIT/SCM_INTPTR_T_BIT. Sure, Guile assumes that intptr=size_t elsewhere, but it's an assumption that can trivially be avoided here. Now, on to libguile/bytevectors.c: +#if !(__MINGW32__ && __x86_64__) #define is_signed_int32(_x) (((_x) >= -2147483648L) && ((_x) <= 2147483647L)) #define is_unsigned_int32(_x) ((_x) <= 4294967295UL) +#else /* (__MINGW32__ && __x86_64__) */ +#define is_signed_int32(_x) (((_x) >= -2147483648LL) && ((_x) <= 2147483647LL)) +#define is_unsigned_int32(_x) ((_x) <= 4294967295ULL) +#endif /* (__MINGW32__ && __x86_64__) */ These #if / #else suck. What if there exists some other system where long is 32-bit? They are also unnecessary, it is possible to just use LL/ULL unconditionally (Guile already assumes that 'long long' exists, in test-num2integral.c): #define is_signed_int32(_x) (((_x) >= -2147483648LL) && ((_x) <= 2147483647LL)) #define is_unsigned_int32(_x) ((_x) <= 4294967295ULL) #define is_unsigned_int32(_x) ((_x) <= 4294967295ULL) Also, in twos_complement, the implementation is confusingly written -- the MPZ API accepts longs, not uintptr, going by the official documentation. There is a reason for this, but it needs comments for people not aware that Guile replaces the GMP implementation on Windows. About libguile/deprecated.c: this change is unnecessary if the definition of scm_t_inum is fixed. About libguile/hash.c / libguile/hash.h: I don't get the type changes -- if 32-bit hashes have too high risk of collisions, why not also have 64-bit hashes on 32-bit architectures instead of only when uintptr_t is 64-bit? It looks like an overly general search+replace long->intptr_t to me. About libguile/integers.c: +#if !(__MINGW32__ && __x86_64__) +#define L1 1L +#else /* (__MINGW32__ && __x86_64__) */ +#define L1 1LL +#endif /* (__MINGW32__ && __x86_64__) */ Again, these #if/#else suck. Looking at the uses of L1, L1 should be an uintptr_t -- this code doesn't need to know whether L1 is long or long long, it just needs to be an intptr_t. It can be replaced by INTPTR_C(1) instead -- ok, bah, INTPTR_C doesn't actually exist, but you can define it in terms of INT<N>_C and some (cross-platform!!) #if/#else that compare UINTN_MAX with UINTPTR_MAX. (Likewise, the configure.ac test is bad.) Also, because integers.c deals with fixnums, it should be using scm_t_inum instead of intptr_t. About scmsigs.c: unless signal numbers have more than 32-bits on Windows, I don't think that change accomplishes anything. About srfi-60.c: unless the goal was to improve performance by using larger integers, these changes don't seem to accomplish anything. And if the goal is performance, then this needs a benchmark. About strings.h and symbols.c: see my comments about hashes. Overall, I think there are some places were Guile really needs to stop using 'long' and instead use a more specific type like 'size_t' / 'uintptr_t', for example when representing sizes and offsets of array-like stuff, whether or not Guile is ported to Windows. > To get around this, the x86-64-MinGW port now requires the use of > mini-GMP. Mini-GMP has been changed to use intptr_t and uintptr_t. Then the local copy should state that, otherwise people updating Guile's minigmp will introduce bugs. > Let me know what you think. More cross-platform support = good and out-of-tree = inconvenient, but the current patch makes some questionable choices and doesn't document stuff properly; it's really inconvenient for a newcomer from, say, 2050, to have to dig through multiple decades of commits and e-mails to avoid pitfalls like ‘this comment used to be true but now is a lie’ and ‘hidden assumptions’ when doing some refactoring or porting. Also, regardless of whether the uintptr stuff is accepted, there seem to be some Windows-independent patches that seem fit for the main branch: * Avoid mysterious "error no error" message in pipe * fixes for chmodat test (Also, not actually a maintainer myself.) > -Mike Gran > > [1] Last two times I volunteered myself for something on Guile/Guix, > I got quite ill. Let's not hope for a repeat. Ill, like in, bad social interactions, or like in independent medical issues that happened at the same time as working on Guile/Guix stuff? I don't need to know per se but this footnote is ambiguous. Greetings, Maxime. [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 929 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 236 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Guile 64-bit Windows support, redux 2023-06-08 19:50 ` Maxime Devos @ 2023-06-08 20:46 ` Mike Gran 2023-06-09 11:01 ` Maxime Devos 0 siblings, 1 reply; 23+ messages in thread From: Mike Gran @ 2023-06-08 20:46 UTC (permalink / raw) To: guile-devel@gnu.org, Maxime Devos > (The following response is a bit muddled -- it initially says I don't > follow but later I will. Still, there is some stuff of which I really > disagree with the implementation.) > I don't quite follow why the change from 'long' to 'intptr_t'. I mean, > reading the commit message you linked to: Hello Maxime, Thank you for the review, and I'll go through at some point, but, for now I've restarting a new patch set based on what I've learned before and with a higher level of formality. I'm starting with the easy parts and will leave this numerical part until the very end. So it may take some weeks to form a proper reply. Fundamentally, Guile has a habit of putting pointers into longs, so that's really all that needs to be fixed. But is ends up being more complicated that it sounds. But when I get back to it, I'll try to be more precise 1. long or long long - an integer that is "big" in some general sense 2. int64_t - an integer that needs to be 64-bit because may hold a large integer 3. intptr_t - an integer that holds a pointer I saw Ludo in chat and he said that it was important to not change the ABI, so I'll need to create some tooling to ensure that everything stays consistent as I progress. >> >> [1] Last two times I volunteered myself for something on Guile/Guix, >> I got quite ill. Let's not hope for a repeat. > Ill, like in, bad social interactions, or like in independent medical > issues that happened at the same time as working on Guile/Guix stuff? >I don't need to know per se but this footnote is ambiguous. Oh sorry, I didn't imagine it could be read that way. Medically ill. I recommend against getting old, if you can avoid it. Regards, Mike Gran ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Guile 64-bit Windows support, redux 2023-06-08 20:46 ` Mike Gran @ 2023-06-09 11:01 ` Maxime Devos 2023-06-22 13:36 ` Mike Gran 0 siblings, 1 reply; 23+ messages in thread From: Maxime Devos @ 2023-06-09 11:01 UTC (permalink / raw) To: Mike Gran, guile-devel@gnu.org [-- Attachment #1.1.1: Type: text/plain, Size: 330 bytes --] Op 08-06-2023 om 22:46 schreef Mike Gran: > But when I get back to it, I'll try to be more precise > 1. long or long long - an integer that is "big" in some general sense > 2. int64_t - an integer that needs to be 64-bit because may hold a large integer > 3. intptr_t - an integer that holds a pointer That's the idea [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 929 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 236 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Guile 64-bit Windows support, redux 2023-06-09 11:01 ` Maxime Devos @ 2023-06-22 13:36 ` Mike Gran 0 siblings, 0 replies; 23+ messages in thread From: Mike Gran @ 2023-06-22 13:36 UTC (permalink / raw) To: guile-devel@gnu.org, Maxime Devos Hey Guile- So I'm still plugging away at this Guile on Windows stuff. In a tree on Github [1], I started from the top and cleaned up the 64-bit Cygwin and MSYS problems. Cygwin and MSYS are two related projects where you compile with GCC, with the newlib C library, and a link to a library that provides all of POSIX. The POSIX emulation is good, so cleaning up support here is not hard. From there I've spent a couple of weeks stuck on the 32-bit MINGW32 build. MINGW32 is where you compile using GCC but with the Microsoft C library MSVCRT. For a long time now, Guile kind of worked on 32-bit MinGW, but without threads and without JIT. Getting JIT to work ended up being easy, but the threads part has been tough. Guile's use of BDW GC is complicated. It is not the "just replace malloc with GC_MALLOC" described in the BDW GC docs. I'm stuck on a bug where somehow garbage ends up getting passed to the SMOB mark function by the GC, and then SMOB mark function calls abort(). Anyway, I'm down in it. Custom debug versions of libgc and printfs everywhere. But if garbage getting passed to the smob mark function is a known issue that someone has already solved, please let me know. I haven't pushed anything back to the main Guile git repo savannah yet, because I don't want y'all to see me flailing around. Regards, Mike [1] github.com/spk121/guile 'staging' branch ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Guile 64-bit Windows support, redux 2023-06-06 20:50 ` Guile 64-bit Windows support, redux Mike Gran ` (2 preceding siblings ...) 2023-06-08 19:50 ` Maxime Devos @ 2023-10-29 21:34 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 2023-11-28 21:04 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 3 siblings, 1 reply; 23+ messages in thread From: Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library @ 2023-10-29 21:34 UTC (permalink / raw) To: Mike Gran, guile-devel@gnu.org [-- Attachment #1.1: Type: text/plain, Size: 2355 bytes --] On Tue, 2023-06-06 at 20:50 +0000, Mike Gran wrote: > Hello Guile, Hi Mike, I'm sorry for "necrobumping" this thread; I wanted to reply back in June but didn't have enough time to work through what I will explain further below. In any case, I would very much like to see good Windows 64-bit support in Guile to make use of it in LilyPond. > [...] > > I'd be willing [1] to update the patches for MinGW support for > review and inclusion into the main branch > for 3.0.10 or 3.2 but, only if maintainers are open, in principle, > to committing the most disruptive patch: the one where all > long integers become intptr_t to deal with Window's stupid > 4-byte longs on 64-bit OSs. Without agreeing on that as a > possibility, 64-bit Windows support is DOA. For a long time, I wasn't really convinced by this argument until I realized that the alternative would complicate cross-compilation and move away from the simple {32-bit, 64-bit} x {little-endian, big- endian} matrix for the bytecode. > There are a couple of slightly outdated versions of janekke's 64-bit patch. > Here, for example: > > https://git.savannah.gnu.org/cgit/guile.git/commit/?h=wip-mingw&id=76950b4281c7dfff78b9ead6d3d62c070bbc1f13 If I understand correctly, one of the contentious points of this commit (apart from being big and having many changes in one) is that it moves GMP away from long. IIRC there was a statement somewhere that GMP will not change their APIs, which basically means that Guile on Windows would forever be stuck with a quite significantly patched mini-gmp. I would like to propose a different approach: It turns out to be possible to just define scm_t_inum as intptr_t, while leaving GMP interfaces alone (be that mini-gmp or a full GMP). Instead, the mismatch in type widths can be handled during the conversion to mpz. For a practical implementation, see the fourth patch attached to this message. Afterwards, the fifth patch takes care of the hashes, which are expected to have the same width as pointers. This is required because (at least) hashes of symbols are stored into the bytecode. Taken together, this seems to enable enough functionality to run LilyPond with Guile 3 on Windows. What do you and others think of this approach, would this be "more" acceptable to land in main? Jonas [-- Attachment #1.2: 0001-scm_i_divide2double-Refactor-to-use-scm_to_mpz.patch --] [-- Type: text/x-patch, Size: 1343 bytes --] From 996ddfa286434b242f89fae776241c196169a4a1 Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld <hahnjo@hahnjo.de> Date: Wed, 30 Aug 2023 17:07:10 +0200 Subject: [PATCH 1/5] scm_i_divide2double: Refactor to use scm_to_mpz * libguile/numbers.c (scm_i_divide2double): Refactor to use scm_to_mpz. --- libguile/numbers.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/libguile/numbers.c b/libguile/numbers.c index 30a826f13..18fa21b6d 100644 --- a/libguile/numbers.c +++ b/libguile/numbers.c @@ -294,16 +294,11 @@ scm_i_divide2double (SCM n, SCM d) else return 0.0 / 0.0; } - - mpz_init_set_si (dd, SCM_I_INUM (d)); } - else - scm_integer_init_set_mpz_z (scm_bignum (d), dd); - if (SCM_I_INUMP (n)) - mpz_init_set_si (nn, SCM_I_INUM (n)); - else - scm_integer_init_set_mpz_z (scm_bignum (n), nn); + mpz_inits (nn, dd, lo, hi, x, NULL); + scm_to_mpz (d, dd); + scm_to_mpz (n, nn); neg = (mpz_sgn (nn) < 0) ^ (mpz_sgn (dd) < 0); mpz_abs (nn, nn); @@ -351,7 +346,6 @@ scm_i_divide2double (SCM n, SCM d) /* Compute the initial values of lo, x, and hi based on the initial guess of e */ - mpz_inits (lo, hi, x, NULL); mpz_mul_2exp (x, nn, 2 + ((e < 0) ? -e : 0)); mpz_mul (lo, dd, scm_i_divide2double_lo2b); if (e > 0) -- 2.42.0 [-- Attachment #1.3: 0002-scm_integer_modulo_expt_nnn-Refactor-to-use-scm_to_m.patch --] [-- Type: text/x-patch, Size: 1686 bytes --] From 3b94097c6ce0d2c92a156dad5a0c7c43071f23d5 Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld <hahnjo@hahnjo.de> Date: Wed, 30 Aug 2023 17:36:30 +0200 Subject: [PATCH 2/5] scm_integer_modulo_expt_nnn: Refactor to use scm_to_mpz * libguile/integers.c (scm_integer_modulo_expt_nnn): Refactor to use scm_to_mpz. (integer_init_mpz): Remove helper function. --- libguile/integers.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/libguile/integers.c b/libguile/integers.c index cc62d1c78..81ee06206 100644 --- a/libguile/integers.c +++ b/libguile/integers.c @@ -2318,21 +2318,6 @@ scm_integer_expt_zi (struct scm_bignum *n, scm_t_inum k) return take_mpz (res); } -static void -integer_init_mpz (mpz_ptr z, SCM n) -{ - if (SCM_I_INUMP (n)) - mpz_init_set_si (z, SCM_I_INUM (n)); - else - { - ASSERT (SCM_BIGP (n)); - mpz_t zn; - alias_bignum_to_mpz (scm_bignum (n), zn); - mpz_init_set (z, zn); - scm_remember_upto_here_1 (n); - } -} - SCM scm_integer_modulo_expt_nnn (SCM n, SCM k, SCM m) { @@ -2341,9 +2326,16 @@ scm_integer_modulo_expt_nnn (SCM n, SCM k, SCM m) mpz_t n_tmp, k_tmp, m_tmp; - integer_init_mpz (n_tmp, n); - integer_init_mpz (k_tmp, k); - integer_init_mpz (m_tmp, m); +#if (! HAVE_DECL_MPZ_INITS) || SCM_ENABLE_MINI_GMP + mpz_init (n_tmp); + mpz_init (k_tmp); + mpz_init (m_tmp); +#else + mpz_inits (n_tmp, k_tmp, m_tmp, NULL); +#endif + scm_to_mpz (n, n_tmp); + scm_to_mpz (k, k_tmp); + scm_to_mpz (m, m_tmp); /* if the exponent K is negative, and we simply call mpz_powm, we will get a divide-by-zero exception when an inverse 1/n mod m -- 2.42.0 [-- Attachment #1.4: 0003-Rename-functions-that-should-accept-scm_t_inum.patch --] [-- Type: text/x-patch, Size: 6283 bytes --] From 5a6e65e166c4c32bcc255275df2b8276acca1e41 Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld <hahnjo@hahnjo.de> Date: Sat, 2 Sep 2023 16:38:59 +0200 Subject: [PATCH 3/5] Rename functions that should accept scm_t_inum * libguile/integers.c (long_to_bignum): Rename to inum_to_bignum. (long_to_scm): Rename to scm_from_inum. --- libguile/integers.c | 52 ++++++++++++++++++++++----------------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/libguile/integers.c b/libguile/integers.c index 81ee06206..b4090e5bf 100644 --- a/libguile/integers.c +++ b/libguile/integers.c @@ -245,7 +245,7 @@ ulong_to_bignum (unsigned long u) }; static struct scm_bignum * -long_to_bignum (long i) +inum_to_bignum (scm_t_inum i) { if (i > 0) return ulong_to_bignum (i); @@ -260,11 +260,11 @@ scm_from_bignum (struct scm_bignum *x) } static SCM -long_to_scm (long i) +scm_from_inum (scm_t_inum i) { if (SCM_FIXABLE (i)) return SCM_I_MAKINUM (i); - return scm_from_bignum (long_to_bignum (i)); + return scm_from_bignum (inum_to_bignum (i)); } static SCM @@ -328,7 +328,7 @@ take_mpz (mpz_ptr mpz) { SCM ret; if (mpz_fits_slong_p (mpz)) - ret = long_to_scm (mpz_get_si (mpz)); + ret = scm_from_inum (mpz_get_si (mpz)); else ret = scm_from_bignum (make_bignum_from_mpz (mpz)); mpz_clear (mpz); @@ -516,7 +516,7 @@ scm_integer_abs_i (scm_t_inum i) if (i >= 0) return SCM_I_MAKINUM (i); - return ulong_to_scm (long_magnitude (i)); + return scm_from_inum (-i); } SCM @@ -541,7 +541,7 @@ scm_integer_floor_quotient_ii (scm_t_inum x, scm_t_inum y) else if (x > 0) x = x - y - 1; scm_t_inum q = x / y; - return long_to_scm (q); + return scm_from_inum (q); } SCM @@ -675,7 +675,7 @@ scm_integer_floor_divide_ii (scm_t_inum x, scm_t_inum y, SCM *qp, SCM *rp) q--; } - *qp = long_to_scm (q); + *qp = scm_from_inum (q); *rp = SCM_I_MAKINUM (r); } @@ -768,7 +768,7 @@ scm_integer_ceiling_quotient_ii (scm_t_inum x, scm_t_inum y) x = x + y + 1; scm_t_inum q = x / y; - return long_to_scm (q); + return scm_from_inum (q); } SCM @@ -935,7 +935,7 @@ scm_integer_ceiling_divide_ii (scm_t_inum x, scm_t_inum y, SCM *qp, SCM *rp) r -= y; q++; } - *qp = long_to_scm (q); + *qp = scm_from_inum (q); *rp = SCM_I_MAKINUM (r); } } @@ -1034,7 +1034,7 @@ scm_integer_truncate_quotient_ii (scm_t_inum x, scm_t_inum y) else { scm_t_inum q = x / y; - return long_to_scm (q); + return scm_from_inum (q); } } @@ -1096,7 +1096,7 @@ scm_integer_truncate_remainder_ii (scm_t_inum x, scm_t_inum y) else { scm_t_inum q = x % y; - return long_to_scm (q); + return scm_from_inum (q); } } @@ -1150,7 +1150,7 @@ scm_integer_truncate_divide_ii (scm_t_inum x, scm_t_inum y, SCM *qp, SCM *rp) { scm_t_inum q = x / y; scm_t_inum r = x % y; - *qp = long_to_scm (q); + *qp = scm_from_inum (q); *rp = SCM_I_MAKINUM (r); } } @@ -1284,13 +1284,13 @@ scm_integer_centered_quotient_ii (scm_t_inum x, scm_t_inum y) q++; } } - return long_to_scm (q); + return scm_from_inum (q); } SCM scm_integer_centered_quotient_iz (scm_t_inum x, struct scm_bignum *y) { - return integer_centered_quotient_zz (long_to_bignum (x), + return integer_centered_quotient_zz (inum_to_bignum (x), y); } @@ -1409,7 +1409,7 @@ scm_integer_centered_remainder_ii (scm_t_inum x, scm_t_inum y) SCM scm_integer_centered_remainder_iz (scm_t_inum x, struct scm_bignum *y) { - return integer_centered_remainder_zz (long_to_bignum (x), + return integer_centered_remainder_zz (inum_to_bignum (x), y); } @@ -1525,14 +1525,14 @@ scm_integer_centered_divide_ii (scm_t_inum x, scm_t_inum y, SCM *qp, SCM *rp) { q++; r -= y; } } } - *qp = long_to_scm (q); + *qp = scm_from_inum (q); *rp = SCM_I_MAKINUM (r); } void scm_integer_centered_divide_iz (scm_t_inum x, struct scm_bignum *y, SCM *qp, SCM *rp) { - integer_centered_divide_zz (long_to_bignum (x), y, qp, rp); + integer_centered_divide_zz (inum_to_bignum (x), y, qp, rp); } void @@ -1643,13 +1643,13 @@ scm_integer_round_quotient_ii (scm_t_inum x, scm_t_inum y) else if (r2 < -ay) q--; } - return long_to_scm (q); + return scm_from_inum (q); } SCM scm_integer_round_quotient_iz (scm_t_inum x, struct scm_bignum *y) { - return integer_round_quotient_zz (long_to_bignum (x), y); + return integer_round_quotient_zz (inum_to_bignum (x), y); } SCM @@ -1789,7 +1789,7 @@ scm_integer_round_remainder_ii (scm_t_inum x, scm_t_inum y) SCM scm_integer_round_remainder_iz (scm_t_inum x, struct scm_bignum *y) { - return integer_round_remainder_zz (long_to_bignum (x), y); + return integer_round_remainder_zz (inum_to_bignum (x), y); } SCM @@ -1902,14 +1902,14 @@ scm_integer_round_divide_ii (scm_t_inum x, scm_t_inum y, SCM *qp, SCM *rp) else if (r2 < -ay) { q--; r += y; } } - *qp = long_to_scm (q); + *qp = scm_from_inum (q); *rp = SCM_I_MAKINUM (r); } void scm_integer_round_divide_iz (scm_t_inum x, struct scm_bignum *y, SCM *qp, SCM *rp) { - integer_round_divide_zz (long_to_bignum (x), y, qp, rp); + integer_round_divide_zz (inum_to_bignum (x), y, qp, rp); } void @@ -2004,7 +2004,7 @@ scm_integer_gcd_ii (scm_t_inum x, scm_t_inum y) } result = u << k; } - return ulong_to_scm (result); + return scm_from_inum (result); } SCM @@ -2831,7 +2831,7 @@ scm_integer_from_double (double val) SCM scm_integer_add_ii (scm_t_inum x, scm_t_inum y) { - return long_to_scm (x + y); + return scm_from_inum (x + y); } static SCM @@ -2949,7 +2949,7 @@ scm_integer_add_zz (struct scm_bignum *x, struct scm_bignum *y) SCM scm_integer_negate_i (scm_t_inum x) { - return long_to_scm (-x); + return scm_from_inum (-x); } SCM @@ -3229,7 +3229,7 @@ scm_integer_from_int32 (int32_t n) { if (SCM_FIXABLE (n)) return SCM_I_MAKINUM (n); - return scm_from_bignum (long_to_bignum (n)); + return scm_from_bignum (inum_to_bignum (n)); } SCM -- 2.42.0 [-- Attachment #1.5: 0004-Decouple-scm_t_inum-from-long-datatype.patch --] [-- Type: text/x-patch, Size: 5084 bytes --] From 54e47a5c63b7e6a85522b158ed514ddf931a556e Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld <hahnjo@hahnjo.de> Date: Sat, 2 Sep 2023 16:15:37 +0200 Subject: [PATCH 4/5] Decouple scm_t_inum from long datatype Guile expects that scm_t_inum (a typedef to long before this patch) has the same size as pointers to get compatible bytecode on different platforms. This assumption breaks on 64-bit Windows where longs are still 32 bit. Instead use intptr_t as the underlying datatype. Unfortunately, this comes with an additional challenge because GMP functions accept unsigned longs as arguments. So instead, in such configurations where long < scm_t_inum, split the values into two longs to convert to mpz. * libguile/scm.h: Define SCM_INTPTR_T_BIT. * libguile/numbers.h (scm_t_inum): Typedef to intptr_t. Update the definitions of SCM_I_FIXNUM_BIT and SCM_MOST_NEGATIVE_FIXNUM. * libguile/numbers.c: Update verify. (scm_to_mpz): Implement if SCM_LONG_BIT < SCM_I_FIXNUM_BIT. * libguile/integers.c (inum_to_bignum, scm_integer_gcd_zi): Implement if SCM_LONG_BIT < SCM_I_FIXNUM_BIT. --- libguile/integers.c | 12 ++++++++++++ libguile/numbers.c | 25 ++++++++++++++++++++++--- libguile/numbers.h | 10 ++++------ libguile/scm.h | 2 ++ 4 files changed, 40 insertions(+), 9 deletions(-) diff --git a/libguile/integers.c b/libguile/integers.c index b4090e5bf..23bd2c0d5 100644 --- a/libguile/integers.c +++ b/libguile/integers.c @@ -247,10 +247,14 @@ ulong_to_bignum (unsigned long u) static struct scm_bignum * inum_to_bignum (scm_t_inum i) { +#if SCM_LONG_BIT >= SCM_I_FIXNUM_BIT if (i > 0) return ulong_to_bignum (i); return i == 0 ? make_bignum_0 () : make_bignum_1 (1, long_magnitude (i)); +#else + return make_bignum_from_int64 (i); +#endif }; static inline SCM @@ -2015,6 +2019,14 @@ scm_integer_gcd_zi (struct scm_bignum *x, scm_t_inum y) return scm_integer_abs_z (x); if (y < 0) y = -y; +#if SCM_I_FIXNUM_BIT > SCM_LONG_BIT + if (y > ULONG_MAX) + { + struct scm_bignum *y_bignum = inum_to_bignum (y); + return scm_integer_gcd_zz (x, y_bignum); + } +#endif + mpz_t zx; alias_bignum_to_mpz (x, zx); result = mpz_gcd_ui (NULL, zx, y); diff --git a/libguile/numbers.c b/libguile/numbers.c index 18fa21b6d..92c202c83 100644 --- a/libguile/numbers.c +++ b/libguile/numbers.c @@ -94,10 +94,10 @@ verify (FLT_RADIX == 2); /* Make sure that scm_t_inum fits within a SCM value. */ verify (sizeof (scm_t_inum) <= sizeof (scm_t_bits)); -/* Several functions below assume that fixnums fit within a long, and +/* Several functions below assume that fixnums fit within an intptr_t, and furthermore that there is some headroom to spare for other operations without overflowing. */ -verify (SCM_I_FIXNUM_BIT <= SCM_LONG_BIT - 2); +verify (SCM_I_FIXNUM_BIT <= SCM_INTPTR_T_BIT - 2); /* Some functions that use GMP's mpn functions assume that a non-negative fixnum will always fit in a 'mp_limb_t'. */ @@ -6857,7 +6857,26 @@ void scm_to_mpz (SCM val, mpz_t rop) { if (SCM_I_INUMP (val)) - mpz_set_si (rop, SCM_I_INUM (val)); + { + scm_t_inum inum = SCM_I_INUM (val); +#if SCM_LONG_BIT >= SCM_I_FIXNUM_BIT + // Cast to long and directly pass to GMP. + mpz_set_si (rop, (long)inum); +#elif (2 * SCM_LONG_BIT) > SCM_I_FIXNUM_BIT + scm_t_inum inum_abs = inum; + if (inum < 0) + inum_abs *= -1; + long high = inum_abs >> (SCM_LONG_BIT - 1); + long low = (long)(inum_abs & ((((scm_t_inum)1) << (SCM_LONG_BIT - 1)) - 1)); + mpz_set_si (rop, high); + mpz_mul_2exp (rop, rop, SCM_LONG_BIT - 1); + mpz_add_ui (rop, rop, low); + if (inum < 0) + mpz_neg (rop, rop); +#else +#error Unknown configuration +#endif + } else if (SCM_BIGP (val)) scm_integer_set_mpz_z (scm_bignum (val), rop); else diff --git a/libguile/numbers.h b/libguile/numbers.h index 84ad5466f..8bc87829a 100644 --- a/libguile/numbers.h +++ b/libguile/numbers.h @@ -52,12 +52,10 @@ extern "C++" { * * Inums are exact integers that fit within an SCM word * (along with two tagging bits). - * - * In the current implementation, Inums must also fit within a long - * because that's what GMP's mpz_*_si functions accept. */ -typedef long scm_t_inum; -#define SCM_I_FIXNUM_BIT (SCM_LONG_BIT - 2) -#define SCM_MOST_NEGATIVE_FIXNUM (-1L << (SCM_I_FIXNUM_BIT - 1)) + */ +typedef intptr_t scm_t_inum; +#define SCM_I_FIXNUM_BIT (SCM_INTPTR_T_BIT - 2) +#define SCM_MOST_NEGATIVE_FIXNUM (((scm_t_inum) -1) << (SCM_I_FIXNUM_BIT - 1)) #define SCM_MOST_POSITIVE_FIXNUM (- (SCM_MOST_NEGATIVE_FIXNUM + 1)) /* SCM_SRS (X, Y) is signed right shift, defined as floor (X / 2^Y), diff --git a/libguile/scm.h b/libguile/scm.h index 4d079b1a8..e053c9883 100644 --- a/libguile/scm.h +++ b/libguile/scm.h @@ -843,6 +843,8 @@ typedef struct scm_thread scm_thread; # define SCM_LONG_BIT (SCM_SIZEOF_LONG * 8) #endif +#define SCM_INTPTR_T_BIT (SCM_SIZEOF_INTPTR_T * 8) + \f /* Cast pointer through (void *) in order to avoid compiler warnings -- 2.42.0 [-- Attachment #1.6: 0005-Store-hashes-as-uintptr_t.patch --] [-- Type: text/x-patch, Size: 14081 bytes --] From 639d9a53c883ec310783ba8ed9b43a485c0e5b61 Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld <hahnjo@hahnjo.de> Date: Tue, 24 Oct 2023 23:47:41 +0200 Subject: [PATCH 5/5] Store hashes as uintptr_t As for scm_t_inum, Guile expects that hashes have the same size as pointers to get compatible bytecode (with respect to interned symbols) on different platforms. This assumption breaks on 64-bit Windows where longs are still 32 bit. Instead use uintptr_t as the datatype. Based on changes by Jan Nieuwenhuizen, Mike Gran, and Andy Wingo. * libguile/hash.c: * libguile/hash.h: * libguile/strings.c: * libguile/strings.h: * libguile/symbols.c: * libguile/symbols.h: Use uintptr_t to store hashes. --- libguile/hash.c | 68 +++++++++++++++++++++++----------------------- libguile/hash.h | 22 +++++++-------- libguile/strings.c | 2 +- libguile/strings.h | 2 +- libguile/symbols.c | 24 ++++++++-------- libguile/symbols.h | 6 ++-- 6 files changed, 62 insertions(+), 62 deletions(-) diff --git a/libguile/hash.c b/libguile/hash.c index 5abdfe397..cfe14bf1d 100644 --- a/libguile/hash.c +++ b/libguile/hash.c @@ -112,31 +112,31 @@ extern double floor(); the hash on a 64-bit system are equal to the hash on a 32-bit \ system. The low 32 bits just add more entropy. */ \ if (sizeof (ret) == 8) \ - ret = (((unsigned long) c) << 32) | b; \ + ret = (((uintptr_t) c) << 32) | b; \ else \ ret = c; \ } while (0) -static unsigned long +static uintptr_t narrow_string_hash (const uint8_t *str, size_t len) { - unsigned long ret; + uintptr_t ret; JENKINS_LOOKUP3_HASHWORD2 (str, len, ret); ret >>= 2; /* Ensure that it fits in a fixnum. */ return ret; } -static unsigned long +static uintptr_t wide_string_hash (const scm_t_wchar *str, size_t len) { - unsigned long ret; + uintptr_t ret; JENKINS_LOOKUP3_HASHWORD2 (str, len, ret); ret >>= 2; /* Ensure that it fits in a fixnum. */ return ret; } -unsigned long +uintptr_t scm_i_string_hash (SCM str) { size_t len = scm_i_string_length (str); @@ -148,13 +148,13 @@ scm_i_string_hash (SCM str) return wide_string_hash (scm_i_string_wide_chars (str), len); } -unsigned long +uintptr_t scm_i_locale_string_hash (const char *str, size_t len) { return scm_i_string_hash (scm_from_locale_stringn (str, len)); } -unsigned long +uintptr_t scm_i_latin1_string_hash (const char *str, size_t len) { if (len == (size_t) -1) @@ -164,11 +164,11 @@ scm_i_latin1_string_hash (const char *str, size_t len) } /* A tricky optimization, but probably worth it. */ -unsigned long +uintptr_t scm_i_utf8_string_hash (const char *str, size_t len) { const uint8_t *end, *ustr = (const uint8_t *) str; - unsigned long ret; + uintptr_t ret; /* The length of the string in characters. This name corresponds to Jenkins' original name. */ @@ -219,8 +219,8 @@ scm_i_utf8_string_hash (const char *str, size_t len) final (a, b, c); - if (sizeof (unsigned long) == 8) - ret = (((unsigned long) c) << 32) | b; + if (sizeof (uintptr_t) == 8) + ret = (((uintptr_t) c) << 32) | b; else ret = c; @@ -228,16 +228,16 @@ scm_i_utf8_string_hash (const char *str, size_t len) return ret; } -static unsigned long scm_raw_ihashq (scm_t_bits key); -static unsigned long scm_raw_ihash (SCM obj, size_t depth); +static uintptr_t scm_raw_ihashq (scm_t_bits key); +static uintptr_t scm_raw_ihash (SCM obj, size_t depth); /* Return the hash of struct OBJ. Traverse OBJ's fields to compute the result, unless DEPTH is zero. Assumes that OBJ is a struct. */ -static unsigned long +static uintptr_t scm_i_struct_hash (SCM obj, size_t depth) { size_t struct_size, field_num; - unsigned long hash; + uintptr_t hash; struct_size = SCM_STRUCT_SIZE (obj); @@ -257,7 +257,7 @@ scm_i_struct_hash (SCM obj, size_t depth) /* Thomas Wang's integer hasher, from http://www.cris.com/~Ttwang/tech/inthash.htm. */ -static unsigned long +static uintptr_t scm_raw_ihashq (scm_t_bits key) { if (sizeof (key) < 8) @@ -283,7 +283,7 @@ scm_raw_ihashq (scm_t_bits key) } /* `depth' is used to limit recursion. */ -static unsigned long +static uintptr_t scm_raw_ihash (SCM obj, size_t depth) { if (SCM_IMP (obj)) @@ -301,7 +301,7 @@ scm_raw_ihash (SCM obj, size_t depth) SCM n = SCM_I_MAKINUM (SCM_MOST_POSITIVE_FIXNUM); if (scm_is_inexact (obj)) obj = scm_inexact_to_exact (obj); - return scm_raw_ihashq (scm_to_ulong (scm_modulo (obj, n))); + return scm_raw_ihashq (scm_to_uintptr_t (scm_modulo (obj, n))); } else return scm_i_string_hash (scm_number_to_string (obj, scm_from_int (10))); @@ -318,7 +318,7 @@ scm_raw_ihash (SCM obj, size_t depth) { size_t len = SCM_SIMPLE_VECTOR_LENGTH (obj); size_t i = depth / 2; - unsigned long h = scm_raw_ihashq (SCM_CELL_WORD_0 (obj)); + uintptr_t h = scm_raw_ihashq (SCM_CELL_WORD_0 (obj)); if (len) while (i--) h ^= scm_raw_ihash (scm_c_vector_ref (obj, h % len), i); @@ -326,7 +326,7 @@ scm_raw_ihash (SCM obj, size_t depth) } case scm_tc7_syntax: { - unsigned long h; + uintptr_t h; h = scm_raw_ihash (scm_syntax_expression (obj), depth); h ^= scm_raw_ihash (scm_syntax_wrap (obj), depth); h ^= scm_raw_ihash (scm_syntax_module (obj), depth); @@ -365,8 +365,8 @@ scm_raw_ihash (SCM obj, size_t depth) \f -unsigned long -scm_ihashq (SCM obj, unsigned long n) +uintptr_t +scm_ihashq (SCM obj, uintptr_t n) { return scm_raw_ihashq (SCM_UNPACK (obj)) % n; } @@ -386,8 +386,8 @@ SCM_DEFINE (scm_hashq, "hashq", 2, 0, 0, "different values, since @code{foo} will be garbage collected.") #define FUNC_NAME s_scm_hashq { - unsigned long sz = scm_to_unsigned_integer (size, 1, ULONG_MAX); - return scm_from_ulong (scm_ihashq (key, sz)); + uintptr_t sz = scm_to_unsigned_integer (size, 1, UINTPTR_MAX); + return scm_from_unsigned_integer (scm_ihashq (key, sz)); } #undef FUNC_NAME @@ -395,8 +395,8 @@ SCM_DEFINE (scm_hashq, "hashq", 2, 0, 0, \f -unsigned long -scm_ihashv (SCM obj, unsigned long n) +uintptr_t +scm_ihashv (SCM obj, uintptr_t n) { if (SCM_NUMP(obj)) return scm_raw_ihash (obj, 10) % n; @@ -419,8 +419,8 @@ SCM_DEFINE (scm_hashv, "hashv", 2, 0, 0, "different values, since @code{foo} will be garbage collected.") #define FUNC_NAME s_scm_hashv { - unsigned long sz = scm_to_unsigned_integer (size, 1, ULONG_MAX); - return scm_from_ulong (scm_ihashv (key, sz)); + uintptr_t sz = scm_to_unsigned_integer (size, 1, UINTPTR_MAX); + return scm_from_unsigned_integer (scm_ihashv (key, sz)); } #undef FUNC_NAME @@ -428,10 +428,10 @@ SCM_DEFINE (scm_hashv, "hashv", 2, 0, 0, \f -unsigned long -scm_ihash (SCM obj, unsigned long n) +uintptr_t +scm_ihash (SCM obj, uintptr_t n) { - return (unsigned long) scm_raw_ihash (obj, 10) % n; + return scm_raw_ihash (obj, 10) % n; } SCM_DEFINE (scm_hash, "hash", 2, 0, 0, @@ -442,8 +442,8 @@ SCM_DEFINE (scm_hash, "hash", 2, 0, 0, "integer in the range 0 to @var{size} - 1.") #define FUNC_NAME s_scm_hash { - unsigned long sz = scm_to_unsigned_integer (size, 1, ULONG_MAX); - return scm_from_ulong (scm_ihash (key, sz)); + uintptr_t sz = scm_to_unsigned_integer (size, 1, UINTPTR_MAX); + return scm_from_unsigned_integer (scm_ihash (key, sz)); } #undef FUNC_NAME diff --git a/libguile/hash.h b/libguile/hash.h index 0e82b4afc..580d2ce93 100644 --- a/libguile/hash.h +++ b/libguile/hash.h @@ -26,19 +26,19 @@ \f -SCM_INTERNAL unsigned long scm_i_locale_string_hash (const char *str, - size_t len); -SCM_INTERNAL unsigned long scm_i_latin1_string_hash (const char *str, - size_t len); -SCM_INTERNAL unsigned long scm_i_utf8_string_hash (const char *str, - size_t len); - -SCM_INTERNAL unsigned long scm_i_string_hash (SCM str); -SCM_API unsigned long scm_ihashq (SCM obj, unsigned long n); +SCM_INTERNAL uintptr_t scm_i_locale_string_hash (const char *str, + size_t len); +SCM_INTERNAL uintptr_t scm_i_latin1_string_hash (const char *str, + size_t len); +SCM_INTERNAL uintptr_t scm_i_utf8_string_hash (const char *str, + size_t len); + +SCM_INTERNAL uintptr_t scm_i_string_hash (SCM str); +SCM_API uintptr_t scm_ihashq (SCM obj, uintptr_t n); SCM_API SCM scm_hashq (SCM obj, SCM n); -SCM_API unsigned long scm_ihashv (SCM obj, unsigned long n); +SCM_API uintptr_t scm_ihashv (SCM obj, uintptr_t n); SCM_API SCM scm_hashv (SCM obj, SCM n); -SCM_API unsigned long scm_ihash (SCM obj, unsigned long n); +SCM_API uintptr_t scm_ihash (SCM obj, uintptr_t n); SCM_API SCM scm_hash (SCM obj, SCM n); SCM_INTERNAL void scm_init_hash (void); diff --git a/libguile/strings.c b/libguile/strings.c index 5eebb3300..572c554c3 100644 --- a/libguile/strings.c +++ b/libguile/strings.c @@ -760,7 +760,7 @@ scm_i_string_set_x (SCM str, size_t p, scm_t_wchar chr) #define SYMBOL_STRINGBUF SCM_CELL_OBJECT_1 SCM -scm_i_make_symbol (SCM name, scm_t_bits flags, unsigned long hash) +scm_i_make_symbol (SCM name, scm_t_bits flags, uintptr_t hash) { SCM buf, symbol; size_t start, length = STRING_LENGTH (name); diff --git a/libguile/strings.h b/libguile/strings.h index f28ef3246..aa6a601be 100644 --- a/libguile/strings.h +++ b/libguile/strings.h @@ -250,7 +250,7 @@ SCM_INTERNAL void scm_i_string_set_x (SCM str, size_t p, scm_t_wchar chr); /* internal functions related to symbols. */ SCM_INTERNAL SCM scm_i_make_symbol (SCM name, scm_t_bits flags, - unsigned long hash); + uintptr_t hash); SCM_INTERNAL const char *scm_i_symbol_chars (SCM sym); SCM_INTERNAL const scm_t_wchar *scm_i_symbol_wide_chars (SCM sym); SCM_INTERNAL size_t scm_i_symbol_length (SCM sym); diff --git a/libguile/symbols.c b/libguile/symbols.c index 292941e9d..b3ddab67d 100644 --- a/libguile/symbols.c +++ b/libguile/symbols.c @@ -71,8 +71,8 @@ SCM_DEFINE (scm_sys_symbols, "%symbols", 0, 0, 0, /* {Symbols} */ -unsigned long -scm_i_hash_symbol (SCM obj, unsigned long n, void *closure) +uintptr_t +scm_i_hash_symbol (SCM obj, uintptr_t n, void *closure) { return scm_i_symbol_hash (obj) % n; } @@ -80,7 +80,7 @@ scm_i_hash_symbol (SCM obj, unsigned long n, void *closure) struct string_lookup_data { SCM string; - unsigned long string_hash; + uintptr_t string_hash; }; static int @@ -102,7 +102,7 @@ string_lookup_predicate_fn (SCM sym, void *closure) } static SCM -lookup_interned_symbol (SCM name, unsigned long raw_hash) +lookup_interned_symbol (SCM name, uintptr_t raw_hash) { struct string_lookup_data data; @@ -118,7 +118,7 @@ struct latin1_lookup_data { const char *str; size_t len; - unsigned long string_hash; + uintptr_t string_hash; }; static int @@ -134,7 +134,7 @@ latin1_lookup_predicate_fn (SCM sym, void *closure) static SCM lookup_interned_latin1_symbol (const char *str, size_t len, - unsigned long raw_hash) + uintptr_t raw_hash) { struct latin1_lookup_data data; @@ -151,7 +151,7 @@ struct utf8_lookup_data { const char *str; size_t len; - unsigned long string_hash; + uintptr_t string_hash; }; static int @@ -201,7 +201,7 @@ utf8_lookup_predicate_fn (SCM sym, void *closure) static SCM lookup_interned_utf8_symbol (const char *str, size_t len, - unsigned long raw_hash) + uintptr_t raw_hash) { struct utf8_lookup_data data; @@ -239,7 +239,7 @@ static SCM scm_i_str2symbol (SCM str) { SCM symbol; - unsigned long raw_hash = scm_i_string_hash (str); + uintptr_t raw_hash = scm_i_string_hash (str); symbol = lookup_interned_symbol (str, raw_hash); if (scm_is_true (symbol)) @@ -261,7 +261,7 @@ scm_i_str2symbol (SCM str) static SCM scm_i_str2uninterned_symbol (SCM str) { - unsigned long raw_hash = scm_i_string_hash (str); + uintptr_t raw_hash = scm_i_string_hash (str); return scm_i_make_symbol (str, SCM_I_F_SYMBOL_UNINTERNED, raw_hash); } @@ -416,7 +416,7 @@ scm_from_latin1_symbol (const char *sym) SCM scm_from_latin1_symboln (const char *sym, size_t len) { - unsigned long hash; + uintptr_t hash; SCM ret; if (len == (size_t) -1) @@ -442,7 +442,7 @@ scm_from_utf8_symbol (const char *sym) SCM scm_from_utf8_symboln (const char *sym, size_t len) { - unsigned long hash; + uintptr_t hash; SCM ret; if (len == (size_t) -1) diff --git a/libguile/symbols.h b/libguile/symbols.h index e8bc3346f..f541f5126 100644 --- a/libguile/symbols.h +++ b/libguile/symbols.h @@ -31,7 +31,7 @@ \f #define scm_is_symbol(x) (SCM_HAS_TYP7 (x, scm_tc7_symbol)) -#define scm_i_symbol_hash(x) ((unsigned long) SCM_CELL_WORD_2 (x)) +#define scm_i_symbol_hash(x) ((uintptr_t) SCM_CELL_WORD_2 (x)) #define scm_i_symbol_is_interned(x) \ (!(SCM_CELL_WORD_0 (x) & SCM_I_F_SYMBOL_UNINTERNED)) @@ -122,8 +122,8 @@ SCM_API SCM scm_take_utf8_symboln (char *sym, size_t len); /* internal functions. */ -SCM_INTERNAL unsigned long scm_i_hash_symbol (SCM obj, unsigned long n, - void *closure); +SCM_INTERNAL uintptr_t scm_i_hash_symbol (SCM obj, uintptr_t n, + void *closure); SCM_INTERNAL void scm_symbols_prehistory (void); SCM_INTERNAL void scm_init_symbols (void); -- 2.42.0 [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: Guile 64-bit Windows support, redux 2023-10-29 21:34 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library @ 2023-11-28 21:04 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 2023-11-28 22:04 ` Dr. Arne Babenhauserheide 2024-01-04 10:40 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 0 siblings, 2 replies; 23+ messages in thread From: Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library @ 2023-11-28 21:04 UTC (permalink / raw) To: Mike Gran, guile-devel@gnu.org [-- Attachment #1: Type: text/plain, Size: 2695 bytes --] On Sun, 2023-10-29 at 22:34 +0100, Jonas Hahnfeld wrote: > On Tue, 2023-06-06 at 20:50 +0000, Mike Gran wrote: > > Hello Guile, > > Hi Mike, > > I'm sorry for "necrobumping" this thread; I wanted to reply back in > June but didn't have enough time to work through what I will explain > further below. In any case, I would very much like to see good Windows > 64-bit support in Guile to make use of it in LilyPond. > > > [...] > > > > I'd be willing [1] to update the patches for MinGW support for > > review and inclusion into the main branch > > for 3.0.10 or 3.2 but, only if maintainers are open, in principle, > > to committing the most disruptive patch: the one where all > > long integers become intptr_t to deal with Window's stupid > > 4-byte longs on 64-bit OSs. Without agreeing on that as a > > possibility, 64-bit Windows support is DOA. > > For a long time, I wasn't really convinced by this argument until I > realized that the alternative would complicate cross-compilation and > move away from the simple {32-bit, 64-bit} x {little-endian, big- > endian} matrix for the bytecode. > > > There are a couple of slightly outdated versions of janekke's 64-bit patch. > > Here, for example: > > > > https://git.savannah.gnu.org/cgit/guile.git/commit/?h=wip-mingw&id=76950b4281c7dfff78b9ead6d3d62c070bbc1f13 > > If I understand correctly, one of the contentious points of this commit > (apart from being big and having many changes in one) is that it moves > GMP away from long. IIRC there was a statement somewhere that GMP will > not change their APIs, which basically means that Guile on Windows > would forever be stuck with a quite significantly patched mini-gmp. > > I would like to propose a different approach: It turns out to be > possible to just define scm_t_inum as intptr_t, while leaving GMP > interfaces alone (be that mini-gmp or a full GMP). Instead, the > mismatch in type widths can be handled during the conversion to mpz. > For a practical implementation, see the fourth patch attached to this > message. > > Afterwards, the fifth patch takes care of the hashes, which are > expected to have the same width as pointers. This is required because > (at least) hashes of symbols are stored into the bytecode. Taken > together, this seems to enable enough functionality to run LilyPond > with Guile 3 on Windows. > > What do you and others think of this approach, would this be "more" > acceptable to land in main? Ping, any comments on this approach? I built binaries for LilyPond 2.25.10 using these patches applied on top of Guile 3.0.9 and the result seems to work fine on Windows. > Jonas [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Guile 64-bit Windows support, redux 2023-11-28 21:04 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library @ 2023-11-28 22:04 ` Dr. Arne Babenhauserheide 2024-01-04 10:40 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 1 sibling, 0 replies; 23+ messages in thread From: Dr. Arne Babenhauserheide @ 2023-11-28 22:04 UTC (permalink / raw) To: Jonas Hahnfeld; +Cc: Mike Gran, guile-devel [-- Attachment #1: Type: text/plain, Size: 951 bytes --] Jonas Hahnfeld via "Developers list for Guile, the GNU extensibility library" <guile-devel@gnu.org> writes: >> What do you and others think of this approach, would this be "more" >> acceptable to land in main? > > Ping, any comments on this approach? I built binaries for LilyPond > 2.25.10 using these patches applied on top of Guile 3.0.9 and the > result seems to work fine on Windows. It sounds very interesting to me, but sadly I’m not in a position to judge what’s acceptable to land. Still I didn’t want to leave you without an answer, because I am very happy to see you work on this! And making life easier for lilypond is great! From what I’ve seen, the maintainers (Ludo and Andy) have been under pretty high load the last months, so that may explain why there was no answer to your email from end of october. Best wishes, Arne -- Unpolitisch sein heißt politisch sein, ohne es zu merken. draketo.de [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 1125 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Guile 64-bit Windows support, redux 2023-11-28 21:04 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 2023-11-28 22:04 ` Dr. Arne Babenhauserheide @ 2024-01-04 10:40 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 2024-02-06 6:44 ` Dr. Arne Babenhauserheide 2024-02-07 14:19 ` Thompson, David 1 sibling, 2 replies; 23+ messages in thread From: Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library @ 2024-01-04 10:40 UTC (permalink / raw) To: Mike Gran, guile-devel@gnu.org [-- Attachment #1: Type: text/plain, Size: 3072 bytes --] On Tue, 2023-11-28 at 22:04 +0100, Jonas Hahnfeld wrote: > On Sun, 2023-10-29 at 22:34 +0100, Jonas Hahnfeld wrote: > > On Tue, 2023-06-06 at 20:50 +0000, Mike Gran wrote: > > > Hello Guile, > > > > Hi Mike, > > > > I'm sorry for "necrobumping" this thread; I wanted to reply back in > > June but didn't have enough time to work through what I will explain > > further below. In any case, I would very much like to see good Windows > > 64-bit support in Guile to make use of it in LilyPond. > > > > > [...] > > > > > > I'd be willing [1] to update the patches for MinGW support for > > > review and inclusion into the main branch > > > for 3.0.10 or 3.2 but, only if maintainers are open, in principle, > > > to committing the most disruptive patch: the one where all > > > long integers become intptr_t to deal with Window's stupid > > > 4-byte longs on 64-bit OSs. Without agreeing on that as a > > > possibility, 64-bit Windows support is DOA. > > > > For a long time, I wasn't really convinced by this argument until I > > realized that the alternative would complicate cross-compilation and > > move away from the simple {32-bit, 64-bit} x {little-endian, big- > > endian} matrix for the bytecode. > > > > > There are a couple of slightly outdated versions of janekke's 64-bit patch. > > > Here, for example: > > > > > > https://git.savannah.gnu.org/cgit/guile.git/commit/?h=wip-mingw&id=76950b4281c7dfff78b9ead6d3d62c070bbc1f13 > > > > If I understand correctly, one of the contentious points of this commit > > (apart from being big and having many changes in one) is that it moves > > GMP away from long. IIRC there was a statement somewhere that GMP will > > not change their APIs, which basically means that Guile on Windows > > would forever be stuck with a quite significantly patched mini-gmp. > > > > I would like to propose a different approach: It turns out to be > > possible to just define scm_t_inum as intptr_t, while leaving GMP > > interfaces alone (be that mini-gmp or a full GMP). Instead, the > > mismatch in type widths can be handled during the conversion to mpz. > > For a practical implementation, see the fourth patch attached to this > > message. > > > > Afterwards, the fifth patch takes care of the hashes, which are > > expected to have the same width as pointers. This is required because > > (at least) hashes of symbols are stored into the bytecode. Taken > > together, this seems to enable enough functionality to run LilyPond > > with Guile 3 on Windows. > > > > What do you and others think of this approach, would this be "more" > > acceptable to land in main? > > Ping, any comments on this approach? I built binaries for LilyPond > 2.25.10 using these patches applied on top of Guile 3.0.9 and the > result seems to work fine on Windows. Another ping; meanwhile we switched to building the official binaries of LilyPond with Guile 3.0 starting from version 2.25.11, but it would be really great to get rid of our downstream patches... Cheers, Jonas [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Guile 64-bit Windows support, redux 2024-01-04 10:40 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library @ 2024-02-06 6:44 ` Dr. Arne Babenhauserheide 2024-02-07 14:19 ` Thompson, David 1 sibling, 0 replies; 23+ messages in thread From: Dr. Arne Babenhauserheide @ 2024-02-06 6:44 UTC (permalink / raw) To: Jonas Hahnfeld; +Cc: Mike Gran, guile-devel [-- Attachment #1: Type: text/plain, Size: 1823 bytes --] Jonas Hahnfeld via "Developers list for Guile, the GNU extensibility library" <guile-devel@gnu.org> writes: > On Tue, 2023-11-28 at 22:04 +0100, Jonas Hahnfeld wrote: >> On Sun, 2023-10-29 at 22:34 +0100, Jonas Hahnfeld wrote: >> > I would like to propose a different approach: It turns out to be >> > possible to just define scm_t_inum as intptr_t, while leaving GMP >> > interfaces alone (be that mini-gmp or a full GMP). Instead, the >> > mismatch in type widths can be handled during the conversion to mpz. >> > For a practical implementation, see the fourth patch attached to this >> > message. >> > >> > Afterwards, the fifth patch takes care of the hashes, which are >> > expected to have the same width as pointers. This is required because >> > (at least) hashes of symbols are stored into the bytecode. Taken >> > together, this seems to enable enough functionality to run LilyPond >> > with Guile 3 on Windows. >> > >> > What do you and others think of this approach, would this be "more" >> > acceptable to land in main? >> >> Ping, any comments on this approach? I built binaries for LilyPond >> 2.25.10 using these patches applied on top of Guile 3.0.9 and the >> result seems to work fine on Windows. > > Another ping; meanwhile we switched to building the official binaries > of LilyPond with Guile 3.0 starting from version 2.25.11, That’s awesome! > but it would be really great to get rid of our downstream patches... I don’t know enough about our GMP use and cross-compilation, so I cannot really comment on your patch. I can get it merged once it’s reviewed, so if anyone here can review: if this passes review, upstreaming won’t be the bottleneck. Best wishes, Arne -- Unpolitisch sein heißt politisch sein, ohne es zu merken. draketo.de [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 1125 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Guile 64-bit Windows support, redux 2024-01-04 10:40 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 2024-02-06 6:44 ` Dr. Arne Babenhauserheide @ 2024-02-07 14:19 ` Thompson, David 2024-02-07 20:19 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 1 sibling, 1 reply; 23+ messages in thread From: Thompson, David @ 2024-02-07 14:19 UTC (permalink / raw) To: Jonas Hahnfeld; +Cc: Mike Gran, guile-devel@gnu.org On Thu, Jan 4, 2024 at 5:40 AM Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library <guile-devel@gnu.org> wrote: > > On Tue, 2023-11-28 at 22:04 +0100, Jonas Hahnfeld wrote: > > > > Ping, any comments on this approach? I built binaries for LilyPond > > 2.25.10 using these patches applied on top of Guile 3.0.9 and the > > result seems to work fine on Windows. > > Another ping; meanwhile we switched to building the official binaries > of LilyPond with Guile 3.0 starting from version 2.25.11, but it would > be really great to get rid of our downstream patches... Just chiming in to say this is a very exciting development that I had missed when the patch set was first sent! Does this allow a fully featured Guile build or are some things still disabled? Does JIT work? Hopefully Ludo or Andy can spare some time to review soon! - Dave ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Guile 64-bit Windows support, redux 2024-02-07 14:19 ` Thompson, David @ 2024-02-07 20:19 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 2024-02-07 20:23 ` Thompson, David 0 siblings, 1 reply; 23+ messages in thread From: Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library @ 2024-02-07 20:19 UTC (permalink / raw) To: Thompson, David; +Cc: Mike Gran, guile-devel@gnu.org [-- Attachment #1: Type: text/plain, Size: 1595 bytes --] On Wed, 2024-02-07 at 09:19 -0500, Thompson, David wrote: > On Thu, Jan 4, 2024 at 5:40 AM Jonas Hahnfeld via Developers list for > Guile, the GNU extensibility library <guile-devel@gnu.org> wrote: > > > > On Tue, 2023-11-28 at 22:04 +0100, Jonas Hahnfeld wrote: > > > > > > Ping, any comments on this approach? I built binaries for LilyPond > > > 2.25.10 using these patches applied on top of Guile 3.0.9 and the > > > result seems to work fine on Windows. > > > > Another ping; meanwhile we switched to building the official binaries > > of LilyPond with Guile 3.0 starting from version 2.25.11, but it would > > be really great to get rid of our downstream patches... > > Just chiming in to say this is a very exciting development that I had > missed when the patch set was first sent! > > Does this allow a fully featured Guile build or are some things still > disabled? Does JIT work? It's functional enough to run LilyPond (which uses quite a bit of Guile) and well enough so that there is only one complaint (that I know of so far) about multiplication with negative numbers not working right. If I remember correctly from quickly having a look, that's related to scm_integer_mul_ii using long_magnitude which doesn't quite work on Windows 64-bit. For LilyPond, we disable some features (JIT, threading, networking; you can look at the full build recipe here: https://gitlab.com/lilypond/lilypond/-/blob/master/release/binaries/lib/dependencies.py#L628 ) and I don't know which of these would work or how much it would take to support them. Jonas [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Guile 64-bit Windows support, redux 2024-02-07 20:19 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library @ 2024-02-07 20:23 ` Thompson, David 2024-02-07 20:29 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 0 siblings, 1 reply; 23+ messages in thread From: Thompson, David @ 2024-02-07 20:23 UTC (permalink / raw) To: Jonas Hahnfeld; +Cc: Mike Gran, guile-devel@gnu.org On Wed, Feb 7, 2024 at 3:19 PM Jonas Hahnfeld <hahnjo@hahnjo.de> wrote: > > On Wed, 2024-02-07 at 09:19 -0500, Thompson, David wrote: > > On Thu, Jan 4, 2024 at 5:40 AM Jonas Hahnfeld via Developers list for > > Guile, the GNU extensibility library <guile-devel@gnu.org> wrote: > > > > > > On Tue, 2023-11-28 at 22:04 +0100, Jonas Hahnfeld wrote: > > > > > > > > Ping, any comments on this approach? I built binaries for LilyPond > > > > 2.25.10 using these patches applied on top of Guile 3.0.9 and the > > > > result seems to work fine on Windows. > > > > > > Another ping; meanwhile we switched to building the official binaries > > > of LilyPond with Guile 3.0 starting from version 2.25.11, but it would > > > be really great to get rid of our downstream patches... > > > > Just chiming in to say this is a very exciting development that I had > > missed when the patch set was first sent! > > > > Does this allow a fully featured Guile build or are some things still > > disabled? Does JIT work? > > It's functional enough to run LilyPond (which uses quite a bit of > Guile) and well enough so that there is only one complaint (that I know > of so far) about multiplication with negative numbers not working > right. If I remember correctly from quickly having a look, that's > related to scm_integer_mul_ii using long_magnitude which doesn't quite > work on Windows 64-bit. For LilyPond, we disable some features (JIT, > threading, networking; you can look at the full build recipe here: > https://gitlab.com/lilypond/lilypond/-/blob/master/release/binaries/lib/dependencies.py#L628 > ) and I don't know which of these would work or how much it would take > to support them. Ah, bummer. That's a lot of disabled features. JIT and threads are must-haves for my use-cases. I guess I'll continue waiting for someone to figure out how to build a fully featured Guile on Windows. Any takers? ;) - Dave ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Guile 64-bit Windows support, redux 2024-02-07 20:23 ` Thompson, David @ 2024-02-07 20:29 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 2024-03-20 20:28 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 0 siblings, 1 reply; 23+ messages in thread From: Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library @ 2024-02-07 20:29 UTC (permalink / raw) To: Thompson, David; +Cc: Mike Gran, guile-devel@gnu.org [-- Attachment #1: Type: text/plain, Size: 2546 bytes --] On Wed, 2024-02-07 at 15:23 -0500, Thompson, David wrote: > On Wed, Feb 7, 2024 at 3:19 PM Jonas Hahnfeld <hahnjo@hahnjo.de> wrote: > > > > On Wed, 2024-02-07 at 09:19 -0500, Thompson, David wrote: > > > On Thu, Jan 4, 2024 at 5:40 AM Jonas Hahnfeld via Developers list for > > > Guile, the GNU extensibility library <guile-devel@gnu.org> wrote: > > > > > > > > On Tue, 2023-11-28 at 22:04 +0100, Jonas Hahnfeld wrote: > > > > > > > > > > Ping, any comments on this approach? I built binaries for LilyPond > > > > > 2.25.10 using these patches applied on top of Guile 3.0.9 and the > > > > > result seems to work fine on Windows. > > > > > > > > Another ping; meanwhile we switched to building the official binaries > > > > of LilyPond with Guile 3.0 starting from version 2.25.11, but it would > > > > be really great to get rid of our downstream patches... > > > > > > Just chiming in to say this is a very exciting development that I had > > > missed when the patch set was first sent! > > > > > > Does this allow a fully featured Guile build or are some things still > > > disabled? Does JIT work? > > > > It's functional enough to run LilyPond (which uses quite a bit of > > Guile) and well enough so that there is only one complaint (that I know > > of so far) about multiplication with negative numbers not working > > right. If I remember correctly from quickly having a look, that's > > related to scm_integer_mul_ii using long_magnitude which doesn't quite > > work on Windows 64-bit. For LilyPond, we disable some features (JIT, > > threading, networking; you can look at the full build recipe here: > > https://gitlab.com/lilypond/lilypond/-/blob/master/release/binaries/lib/dependencies.py#L628 > > ) and I don't know which of these would work or how much it would take > > to support them. > > Ah, bummer. That's a lot of disabled features. JIT and threads are > must-haves for my use-cases. I guess I'll continue waiting for > someone to figure out how to build a fully featured Guile on Windows. > Any takers? ;) Isn't the common wisdom that you should learn to crawl and walk before starting to run? 😉 first we need a booting and working base of Guile before the rest can follow at some point. For LilyPond's binaries, we disable networking and threads on all platforms so these may just work. Regarding the JIT, I think I disabled it right away when I started trying to understand what the issues were, so it's possible that the state is not actually that dire... [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Guile 64-bit Windows support, redux 2024-02-07 20:29 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library @ 2024-03-20 20:28 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 2024-03-20 20:40 ` Thompson, David 2024-07-14 18:30 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 0 siblings, 2 replies; 23+ messages in thread From: Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library @ 2024-03-20 20:28 UTC (permalink / raw) To: Thompson, David Cc: Mike Gran, guile-devel@gnu.org, Ludovic Courtès, Andy Wingo [-- Attachment #1.1: Type: text/plain, Size: 3548 bytes --] On Wed, 2024-02-07 at 21:29 +0100, Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library wrote: > On Wed, 2024-02-07 at 15:23 -0500, Thompson, David wrote: > > On Wed, Feb 7, 2024 at 3:19 PM Jonas Hahnfeld <hahnjo@hahnjo.de> wrote: > > > > > > On Wed, 2024-02-07 at 09:19 -0500, Thompson, David wrote: > > > > On Thu, Jan 4, 2024 at 5:40 AM Jonas Hahnfeld wrote: > > > > > > > > > > On Tue, 2023-11-28 at 22:04 +0100, Jonas Hahnfeld wrote: > > > > > > > > > > > > Ping, any comments on this approach? I built binaries for LilyPond > > > > > > 2.25.10 using these patches applied on top of Guile 3.0.9 and the > > > > > > result seems to work fine on Windows. > > > > > > > > > > Another ping; meanwhile we switched to building the official binaries > > > > > of LilyPond with Guile 3.0 starting from version 2.25.11, but it would > > > > > be really great to get rid of our downstream patches... > > > > > > > > Just chiming in to say this is a very exciting development that I had > > > > missed when the patch set was first sent! > > > > > > > > Does this allow a fully featured Guile build or are some things still > > > > disabled? Does JIT work? > > > > > > It's functional enough to run LilyPond (which uses quite a bit of > > > Guile) and well enough so that there is only one complaint (that I know > > > of so far) about multiplication with negative numbers not working > > > right. If I remember correctly from quickly having a look, that's > > > related to scm_integer_mul_ii using long_magnitude which doesn't quite > > > work on Windows 64-bit. For LilyPond, we disable some features (JIT, > > > threading, networking; you can look at the full build recipe here: > > > https://gitlab.com/lilypond/lilypond/-/blob/master/release/binaries/lib/dependencies.py#L628 > > > ) and I don't know which of these would work or how much it would take > > > to support them. > > > > Ah, bummer. That's a lot of disabled features. JIT and threads are > > must-haves for my use-cases. I guess I'll continue waiting for > > someone to figure out how to build a fully featured Guile on Windows. > > Any takers? ;) > > Isn't the common wisdom that you should learn to crawl and walk before > starting to run? 😉 first we need a booting and working base of Guile > before the rest can follow at some point. For LilyPond's binaries, we > disable networking and threads on all platforms so these may just work. > Regarding the JIT, I think I disabled it right away when I started > trying to understand what the issues were, so it's possible that the > state is not actually that dire... So I can confirm that JIT indeed doesn't work right now on 64-bit MinGW, but it's relatively easy to fix (first patch). In essence lightening was getting the calling convention wrong. Compilation just works --with-threads, as long as bdwgc was built with --enable-threads=posix to override the automatic detection of win32 threading. I haven't tested if it actually works, but there might be a good chance. For the one known issue I mentioned above, the fix is also not too difficult, just being a bit more careful with mixing scm_t_inum and long (see second patch). I'm also explicitly CC'ing Andy and Ludo - we really need a statement by a maintainer whether this can land. From my point of view, it's a clear improvement in terms of supported platforms, plus tested by LilyPond since some time now which is probably one of the bigger "customers". Jonas [-- Attachment #1.2: 0001-Fix-lightening-x86_64-Windows-calling-convention.patch --] [-- Type: text/x-patch, Size: 2711 bytes --] From e701b1583052102a5e1758394e4c3a1fb7565d27 Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld <hahnjo@hahnjo.de> Date: Wed, 20 Mar 2024 20:26:36 +0100 Subject: [PATCH 1/2] Fix lightening x86_64 Windows calling convention * libguile/lightening/lightening/x86.c: * libguile/lightening/lightening/x86.h: Check _WIN64 macro as set for mingw64. --- libguile/lightening/lightening/x86.c | 10 +++++----- libguile/lightening/lightening/x86.h | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libguile/lightening/lightening/x86.c b/libguile/lightening/lightening/x86.c index f8ac4b0b8..6963d90fd 100644 --- a/libguile/lightening/lightening/x86.c +++ b/libguile/lightening/lightening/x86.c @@ -237,7 +237,7 @@ jit_init(jit_state_t *_jit) static const jit_gpr_t abi_gpr_args[] = { #if __X32 /* No GPRs in args. */ -#elif __CYGWIN__ +#elif defined(__CYGWIN__) || defined(_WIN64) _RCX, _RDX, _R8, _R9 #else _RDI, _RSI, _RDX, _RCX, _R8, _R9 @@ -247,7 +247,7 @@ static const jit_gpr_t abi_gpr_args[] = { static const jit_fpr_t abi_fpr_args[] = { #if __X32 /* No FPRs in args. */ -#elif __CYGWIN__ +#elif defined(__CYGWIN__) || defined(_WIN64) _XMM0, _XMM1, _XMM2, _XMM3 #else _XMM0, _XMM1, _XMM2, _XMM3, _XMM4, _XMM5, _XMM6, _XMM7 @@ -317,7 +317,7 @@ reset_abi_arg_iterator(struct abi_arg_iterator *iter, size_t argc, memset(iter, 0, sizeof *iter); iter->argc = argc; iter->args = args; -#if __CYGWIN__ && __X64 +#if (defined(__CYGWIN__) || defined(_WIN64)) && __X64 // Reserve slots on the stack for 4 register parameters (8 bytes each). iter->stack_size = 32; #endif @@ -330,12 +330,12 @@ next_abi_arg(struct abi_arg_iterator *iter, jit_operand_t *arg) enum jit_operand_abi abi = iter->args[iter->arg_idx].abi; if (is_gpr_arg(abi) && iter->gpr_idx < abi_gpr_arg_count) { *arg = jit_operand_gpr (abi, abi_gpr_args[iter->gpr_idx++]); -#ifdef __CYGWIN__ +#if defined(__CYGWIN__) || defined(_WIN64) iter->fpr_idx++; #endif } else if (is_fpr_arg(abi) && iter->fpr_idx < abi_fpr_arg_count) { *arg = jit_operand_fpr (abi, abi_fpr_args[iter->fpr_idx++]); -#ifdef __CYGWIN__ +#if defined(__CYGWIN__) || defined(_WIN64) iter->gpr_idx++; #endif } else { diff --git a/libguile/lightening/lightening/x86.h b/libguile/lightening/lightening/x86.h index 983ebdb8f..7da8c5977 100644 --- a/libguile/lightening/lightening/x86.h +++ b/libguile/lightening/lightening/x86.h @@ -92,7 +92,7 @@ # define JIT_F6 _XMM6 # define JIT_FTMP _XMM7 # define JIT_PLATFORM_CALLEE_SAVE_GPRS JIT_TMP0 -#elif __CYGWIN__ +#elif defined(__CYGWIN__) || defined(_WIN64) # define JIT_R0 _RAX # define JIT_R1 _RCX # define JIT_R2 _RDX -- 2.44.0 [-- Attachment #1.3: 0002-Use-inum_magnitude-for-inums.patch --] [-- Type: text/x-patch, Size: 1975 bytes --] From cf380c1a734acd0d22041c7c875fbffc6b535d2a Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld <hahnjo@hahnjo.de> Date: Wed, 20 Mar 2024 20:57:02 +0100 Subject: [PATCH 2/2] Use inum_magnitude for inums * libguile/integers.c: Call inum_magnitude instead of long_magnitude for scm_t_inum arguments. --- libguile/integers.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/libguile/integers.c b/libguile/integers.c index 23bd2c0d5..380ff193c 100644 --- a/libguile/integers.c +++ b/libguile/integers.c @@ -251,7 +251,7 @@ inum_to_bignum (scm_t_inum i) if (i > 0) return ulong_to_bignum (i); - return i == 0 ? make_bignum_0 () : make_bignum_1 (1, long_magnitude (i)); + return i == 0 ? make_bignum_0 () : make_bignum_1 (1, inum_magnitude (i)); #else return make_bignum_from_int64 (i); #endif @@ -3061,15 +3061,15 @@ scm_integer_mul_ii (scm_t_inum x, scm_t_inum y) return SCM_I_MAKINUM (k); #endif - mp_limb_t xd[1] = { long_magnitude (x) }; + mp_limb_t xd[1] = { inum_magnitude (x) }; mp_limb_t lo; int negative = (x < 0) != (y < 0); - mp_limb_t hi = mpn_mul_1 (&lo, xd, 1, long_magnitude (y)); + mp_limb_t hi = mpn_mul_1 (&lo, xd, 1, inum_magnitude (y)); if (!hi) { if (negative) { - if (lo <= long_magnitude (SCM_MOST_NEGATIVE_FIXNUM)) + if (lo <= inum_magnitude (SCM_MOST_NEGATIVE_FIXNUM)) return SCM_I_MAKINUM (negative_long (lo)); } else if (lo <= SCM_MOST_POSITIVE_FIXNUM) @@ -3100,7 +3100,7 @@ scm_integer_mul_zi (struct scm_bignum *x, scm_t_inum y) struct scm_bignum *result = allocate_bignum (xn + 1); mp_limb_t *rd = bignum_limbs (result); const mp_limb_t *xd = bignum_limbs (x); - mp_limb_t yd = long_magnitude (y); + mp_limb_t yd = inum_magnitude (y); int negate = bignum_is_negative (x) != (y < 0); mp_limb_t hi = mpn_mul_1 (rd, xd, xn, yd); if (hi) -- 2.44.0 [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: Guile 64-bit Windows support, redux 2024-03-20 20:28 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library @ 2024-03-20 20:40 ` Thompson, David 2024-03-23 15:09 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 2024-07-14 18:30 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 1 sibling, 1 reply; 23+ messages in thread From: Thompson, David @ 2024-03-20 20:40 UTC (permalink / raw) To: Jonas Hahnfeld Cc: Mike Gran, guile-devel@gnu.org, Ludovic Courtès, Andy Wingo On Wed, Mar 20, 2024 at 4:29 PM Jonas Hahnfeld <hahnjo@hahnjo.de> wrote: > > So I can confirm that JIT indeed doesn't work right now on 64-bit > MinGW, but it's relatively easy to fix (first patch). In essence > lightening was getting the calling convention wrong. Wow! Have you seen the JIT do its thing (via GUILE_JIT_LOG) or just verified that compilation succeeds when JIT is enabled? Either way, a big step forward. The patch is very simple, too. > Compilation just works --with-threads, as long as bdwgc was built with > --enable-threads=posix to override the automatic detection of win32 > threading. I haven't tested if it actually works, but there might be a > good chance. This is also encouraging! Anyone out there want to run a test using call-with-new-thread? > For the one known issue I mentioned above, the fix is also not too > difficult, just being a bit more careful with mixing scm_t_inum and > long (see second patch). The patch makes sense to me! > I'm also explicitly CC'ing Andy and Ludo - we really need a statement > by a maintainer whether this can land. From my point of view, it's a > clear improvement in terms of supported platforms, plus tested by > LilyPond since some time now which is probably one of the bigger > "customers". +1 on hearing from Andy and/or Ludo about this. Jonas' patches are small and should be relatively quick to review. :) If this stuff lands I will have to find a Windows machine to try it out sometime. Thanks Jonas! - Dave ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Guile 64-bit Windows support, redux 2024-03-20 20:40 ` Thompson, David @ 2024-03-23 15:09 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 2024-03-29 17:20 ` Thompson, David 0 siblings, 1 reply; 23+ messages in thread From: Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library @ 2024-03-23 15:09 UTC (permalink / raw) To: Thompson, David; +Cc: guile-devel@gnu.org [-- Attachment #1: Type: text/plain, Size: 1279 bytes --] On Wed, 2024-03-20 at 16:40 -0400, Thompson, David wrote: > On Wed, Mar 20, 2024 at 4:29 PM Jonas Hahnfeld <hahnjo@hahnjo.de> wrote: > > So I can confirm that JIT indeed doesn't work right now on 64-bit > > MinGW, but it's relatively easy to fix (first patch). In essence > > lightening was getting the calling convention wrong. > > Wow! Have you seen the JIT do its thing (via GUILE_JIT_LOG) or just > verified that compilation succeeds when JIT is enabled? Either way, a > big step forward. The patch is very simple, too. I had only verified that the produced LilyPond executable still worked, but I can now confirm that setting GUILE_JIT_LOG shows that something is happening. I don't have performance data on this yet, I asked the community to test the version on larger inputs. > > Compilation just works --with-threads, as long as bdwgc was built with > > --enable-threads=posix to override the automatic detection of win32 > > threading. I haven't tested if it actually works, but there might be a > > good chance. > > This is also encouraging! Anyone out there want to run a test using > call-with-new-thread? So for the fun, I tried compiling --with-threads again and (call-with- new-thread) seems to return new threads. Cheers Jonas [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Guile 64-bit Windows support, redux 2024-03-23 15:09 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library @ 2024-03-29 17:20 ` Thompson, David 0 siblings, 0 replies; 23+ messages in thread From: Thompson, David @ 2024-03-29 17:20 UTC (permalink / raw) To: Jonas Hahnfeld; +Cc: guile-devel@gnu.org On Sat, Mar 23, 2024 at 11:09 AM Jonas Hahnfeld <hahnjo@hahnjo.de> wrote: > > On Wed, 2024-03-20 at 16:40 -0400, Thompson, David wrote: > > On Wed, Mar 20, 2024 at 4:29 PM Jonas Hahnfeld <hahnjo@hahnjo.de> wrote: > > > So I can confirm that JIT indeed doesn't work right now on 64-bit > > > MinGW, but it's relatively easy to fix (first patch). In essence > > > lightening was getting the calling convention wrong. > > > > Wow! Have you seen the JIT do its thing (via GUILE_JIT_LOG) or just > > verified that compilation succeeds when JIT is enabled? Either way, a > > big step forward. The patch is very simple, too. > > I had only verified that the produced LilyPond executable still worked, > but I can now confirm that setting GUILE_JIT_LOG shows that something > is happening. I don't have performance data on this yet, I asked the > community to test the version on larger inputs. > > > > Compilation just works --with-threads, as long as bdwgc was built with > > > --enable-threads=posix to override the automatic detection of win32 > > > threading. I haven't tested if it actually works, but there might be a > > > good chance. > > > > This is also encouraging! Anyone out there want to run a test using > > call-with-new-thread? > > So for the fun, I tried compiling --with-threads again and (call-with- > new-thread) seems to return new threads. Your patches look *very very* promising, then! I'm excited. :) - Dave ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Guile 64-bit Windows support, redux 2024-03-20 20:28 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 2024-03-20 20:40 ` Thompson, David @ 2024-07-14 18:30 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 2024-07-14 23:10 ` Dr. Arne Babenhauserheide 1 sibling, 1 reply; 23+ messages in thread From: Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library @ 2024-07-14 18:30 UTC (permalink / raw) To: Ludovic Courtès, Andy Wingo Cc: guile-devel@gnu.org, Michael Käppler [-- Attachment #1.1: Type: text/plain, Size: 1265 bytes --] On Wed, 2024-03-20 at 21:28 +0100, Jonas Hahnfeld wrote: > I'm also explicitly CC'ing Andy and Ludo - we really need a statement > by a maintainer whether this can land. From my point of view, it's a > clear improvement in terms of supported platforms, plus tested by > LilyPond since some time now which is probably one of the bigger > "customers". Another ping on this topic... I'm a bit disappointed that Guile 3.0.10 was released without even a comment on this thread by a maintainer after multiple developers and users expressed their interest in seeing a natively working 64-bit Guile on Windows. We continue to carry the patches downstream in LilyPond, and recently Michael Käppler (CC'ed) investigated and provided fixes for some more problems with 32-bit long on Windows that would ideally find their way upstream. To potentially make some progress, maybe we can start by reviewing the refactoring patches that should be less contentious? I'm attaching rebased versions of them that should apply to current main. I'm also including the patch to store hashes in uintptr_t, as also done by Jan Nieuwenhuizen, Mike Gran, and Andy Wingo in the wip-mingw branch, which has no direct relationship to the GMP function discussion. Jonas [-- Attachment #1.2: 0001-scm_i_divide2double-Refactor-to-use-scm_to_mpz.patch --] [-- Type: text/x-patch, Size: 1343 bytes --] From 990148027d855b2de7dcf07a9b5c967a8741ece7 Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld <hahnjo@hahnjo.de> Date: Wed, 30 Aug 2023 17:07:10 +0200 Subject: [PATCH 1/4] scm_i_divide2double: Refactor to use scm_to_mpz * libguile/numbers.c (scm_i_divide2double): Refactor to use scm_to_mpz. --- libguile/numbers.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/libguile/numbers.c b/libguile/numbers.c index ae2aa7766..fc598694a 100644 --- a/libguile/numbers.c +++ b/libguile/numbers.c @@ -294,16 +294,11 @@ scm_i_divide2double (SCM n, SCM d) else return 0.0 / 0.0; } - - mpz_init_set_si (dd, SCM_I_INUM (d)); } - else - scm_integer_init_set_mpz_z (scm_bignum (d), dd); - if (SCM_I_INUMP (n)) - mpz_init_set_si (nn, SCM_I_INUM (n)); - else - scm_integer_init_set_mpz_z (scm_bignum (n), nn); + mpz_inits (nn, dd, lo, hi, x, NULL); + scm_to_mpz (d, dd); + scm_to_mpz (n, nn); neg = (mpz_sgn (nn) < 0) ^ (mpz_sgn (dd) < 0); mpz_abs (nn, nn); @@ -351,7 +346,6 @@ scm_i_divide2double (SCM n, SCM d) /* Compute the initial values of lo, x, and hi based on the initial guess of e */ - mpz_inits (lo, hi, x, NULL); mpz_mul_2exp (x, nn, 2 + ((e < 0) ? -e : 0)); mpz_mul (lo, dd, scm_i_divide2double_lo2b); if (e > 0) -- 2.45.2 [-- Attachment #1.3: 0002-scm_integer_modulo_expt_nnn-Refactor-to-use-scm_to_m.patch --] [-- Type: text/x-patch, Size: 1686 bytes --] From d357a71af0f701957b9350b9cd21272eee079e3c Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld <hahnjo@hahnjo.de> Date: Wed, 30 Aug 2023 17:36:30 +0200 Subject: [PATCH 2/4] scm_integer_modulo_expt_nnn: Refactor to use scm_to_mpz * libguile/integers.c (scm_integer_modulo_expt_nnn): Refactor to use scm_to_mpz. (integer_init_mpz): Remove helper function. --- libguile/integers.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/libguile/integers.c b/libguile/integers.c index cc62d1c78..81ee06206 100644 --- a/libguile/integers.c +++ b/libguile/integers.c @@ -2318,21 +2318,6 @@ scm_integer_expt_zi (struct scm_bignum *n, scm_t_inum k) return take_mpz (res); } -static void -integer_init_mpz (mpz_ptr z, SCM n) -{ - if (SCM_I_INUMP (n)) - mpz_init_set_si (z, SCM_I_INUM (n)); - else - { - ASSERT (SCM_BIGP (n)); - mpz_t zn; - alias_bignum_to_mpz (scm_bignum (n), zn); - mpz_init_set (z, zn); - scm_remember_upto_here_1 (n); - } -} - SCM scm_integer_modulo_expt_nnn (SCM n, SCM k, SCM m) { @@ -2341,9 +2326,16 @@ scm_integer_modulo_expt_nnn (SCM n, SCM k, SCM m) mpz_t n_tmp, k_tmp, m_tmp; - integer_init_mpz (n_tmp, n); - integer_init_mpz (k_tmp, k); - integer_init_mpz (m_tmp, m); +#if (! HAVE_DECL_MPZ_INITS) || SCM_ENABLE_MINI_GMP + mpz_init (n_tmp); + mpz_init (k_tmp); + mpz_init (m_tmp); +#else + mpz_inits (n_tmp, k_tmp, m_tmp, NULL); +#endif + scm_to_mpz (n, n_tmp); + scm_to_mpz (k, k_tmp); + scm_to_mpz (m, m_tmp); /* if the exponent K is negative, and we simply call mpz_powm, we will get a divide-by-zero exception when an inverse 1/n mod m -- 2.45.2 [-- Attachment #1.4: 0003-Rename-functions-that-should-accept-scm_t_inum.patch --] [-- Type: text/x-patch, Size: 6283 bytes --] From 2236c2d1871447da96c04b34a52a8b769c8e9dfe Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld <hahnjo@hahnjo.de> Date: Sat, 2 Sep 2023 16:38:59 +0200 Subject: [PATCH 3/4] Rename functions that should accept scm_t_inum * libguile/integers.c (long_to_bignum): Rename to inum_to_bignum. (long_to_scm): Rename to scm_from_inum. --- libguile/integers.c | 52 ++++++++++++++++++++++----------------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/libguile/integers.c b/libguile/integers.c index 81ee06206..b4090e5bf 100644 --- a/libguile/integers.c +++ b/libguile/integers.c @@ -245,7 +245,7 @@ ulong_to_bignum (unsigned long u) }; static struct scm_bignum * -long_to_bignum (long i) +inum_to_bignum (scm_t_inum i) { if (i > 0) return ulong_to_bignum (i); @@ -260,11 +260,11 @@ scm_from_bignum (struct scm_bignum *x) } static SCM -long_to_scm (long i) +scm_from_inum (scm_t_inum i) { if (SCM_FIXABLE (i)) return SCM_I_MAKINUM (i); - return scm_from_bignum (long_to_bignum (i)); + return scm_from_bignum (inum_to_bignum (i)); } static SCM @@ -328,7 +328,7 @@ take_mpz (mpz_ptr mpz) { SCM ret; if (mpz_fits_slong_p (mpz)) - ret = long_to_scm (mpz_get_si (mpz)); + ret = scm_from_inum (mpz_get_si (mpz)); else ret = scm_from_bignum (make_bignum_from_mpz (mpz)); mpz_clear (mpz); @@ -516,7 +516,7 @@ scm_integer_abs_i (scm_t_inum i) if (i >= 0) return SCM_I_MAKINUM (i); - return ulong_to_scm (long_magnitude (i)); + return scm_from_inum (-i); } SCM @@ -541,7 +541,7 @@ scm_integer_floor_quotient_ii (scm_t_inum x, scm_t_inum y) else if (x > 0) x = x - y - 1; scm_t_inum q = x / y; - return long_to_scm (q); + return scm_from_inum (q); } SCM @@ -675,7 +675,7 @@ scm_integer_floor_divide_ii (scm_t_inum x, scm_t_inum y, SCM *qp, SCM *rp) q--; } - *qp = long_to_scm (q); + *qp = scm_from_inum (q); *rp = SCM_I_MAKINUM (r); } @@ -768,7 +768,7 @@ scm_integer_ceiling_quotient_ii (scm_t_inum x, scm_t_inum y) x = x + y + 1; scm_t_inum q = x / y; - return long_to_scm (q); + return scm_from_inum (q); } SCM @@ -935,7 +935,7 @@ scm_integer_ceiling_divide_ii (scm_t_inum x, scm_t_inum y, SCM *qp, SCM *rp) r -= y; q++; } - *qp = long_to_scm (q); + *qp = scm_from_inum (q); *rp = SCM_I_MAKINUM (r); } } @@ -1034,7 +1034,7 @@ scm_integer_truncate_quotient_ii (scm_t_inum x, scm_t_inum y) else { scm_t_inum q = x / y; - return long_to_scm (q); + return scm_from_inum (q); } } @@ -1096,7 +1096,7 @@ scm_integer_truncate_remainder_ii (scm_t_inum x, scm_t_inum y) else { scm_t_inum q = x % y; - return long_to_scm (q); + return scm_from_inum (q); } } @@ -1150,7 +1150,7 @@ scm_integer_truncate_divide_ii (scm_t_inum x, scm_t_inum y, SCM *qp, SCM *rp) { scm_t_inum q = x / y; scm_t_inum r = x % y; - *qp = long_to_scm (q); + *qp = scm_from_inum (q); *rp = SCM_I_MAKINUM (r); } } @@ -1284,13 +1284,13 @@ scm_integer_centered_quotient_ii (scm_t_inum x, scm_t_inum y) q++; } } - return long_to_scm (q); + return scm_from_inum (q); } SCM scm_integer_centered_quotient_iz (scm_t_inum x, struct scm_bignum *y) { - return integer_centered_quotient_zz (long_to_bignum (x), + return integer_centered_quotient_zz (inum_to_bignum (x), y); } @@ -1409,7 +1409,7 @@ scm_integer_centered_remainder_ii (scm_t_inum x, scm_t_inum y) SCM scm_integer_centered_remainder_iz (scm_t_inum x, struct scm_bignum *y) { - return integer_centered_remainder_zz (long_to_bignum (x), + return integer_centered_remainder_zz (inum_to_bignum (x), y); } @@ -1525,14 +1525,14 @@ scm_integer_centered_divide_ii (scm_t_inum x, scm_t_inum y, SCM *qp, SCM *rp) { q++; r -= y; } } } - *qp = long_to_scm (q); + *qp = scm_from_inum (q); *rp = SCM_I_MAKINUM (r); } void scm_integer_centered_divide_iz (scm_t_inum x, struct scm_bignum *y, SCM *qp, SCM *rp) { - integer_centered_divide_zz (long_to_bignum (x), y, qp, rp); + integer_centered_divide_zz (inum_to_bignum (x), y, qp, rp); } void @@ -1643,13 +1643,13 @@ scm_integer_round_quotient_ii (scm_t_inum x, scm_t_inum y) else if (r2 < -ay) q--; } - return long_to_scm (q); + return scm_from_inum (q); } SCM scm_integer_round_quotient_iz (scm_t_inum x, struct scm_bignum *y) { - return integer_round_quotient_zz (long_to_bignum (x), y); + return integer_round_quotient_zz (inum_to_bignum (x), y); } SCM @@ -1789,7 +1789,7 @@ scm_integer_round_remainder_ii (scm_t_inum x, scm_t_inum y) SCM scm_integer_round_remainder_iz (scm_t_inum x, struct scm_bignum *y) { - return integer_round_remainder_zz (long_to_bignum (x), y); + return integer_round_remainder_zz (inum_to_bignum (x), y); } SCM @@ -1902,14 +1902,14 @@ scm_integer_round_divide_ii (scm_t_inum x, scm_t_inum y, SCM *qp, SCM *rp) else if (r2 < -ay) { q--; r += y; } } - *qp = long_to_scm (q); + *qp = scm_from_inum (q); *rp = SCM_I_MAKINUM (r); } void scm_integer_round_divide_iz (scm_t_inum x, struct scm_bignum *y, SCM *qp, SCM *rp) { - integer_round_divide_zz (long_to_bignum (x), y, qp, rp); + integer_round_divide_zz (inum_to_bignum (x), y, qp, rp); } void @@ -2004,7 +2004,7 @@ scm_integer_gcd_ii (scm_t_inum x, scm_t_inum y) } result = u << k; } - return ulong_to_scm (result); + return scm_from_inum (result); } SCM @@ -2831,7 +2831,7 @@ scm_integer_from_double (double val) SCM scm_integer_add_ii (scm_t_inum x, scm_t_inum y) { - return long_to_scm (x + y); + return scm_from_inum (x + y); } static SCM @@ -2949,7 +2949,7 @@ scm_integer_add_zz (struct scm_bignum *x, struct scm_bignum *y) SCM scm_integer_negate_i (scm_t_inum x) { - return long_to_scm (-x); + return scm_from_inum (-x); } SCM @@ -3229,7 +3229,7 @@ scm_integer_from_int32 (int32_t n) { if (SCM_FIXABLE (n)) return SCM_I_MAKINUM (n); - return scm_from_bignum (long_to_bignum (n)); + return scm_from_bignum (inum_to_bignum (n)); } SCM -- 2.45.2 [-- Attachment #1.5: 0004-Store-hashes-as-uintptr_t.patch --] [-- Type: text/x-patch, Size: 14222 bytes --] From bb96b47cf70c028e8570fb799238bfeeb2bd3b6e Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld <hahnjo@hahnjo.de> Date: Tue, 24 Oct 2023 23:47:41 +0200 Subject: [PATCH 4/4] Store hashes as uintptr_t Guile expects that hashes have the same size as pointers to get compatible bytecode (with respect to interned symbols) on different platforms. This assumption breaks on 64-bit Windows where longs are still 32 bit. Instead use uintptr_t as the datatype. Based on changes by Jan Nieuwenhuizen, Mike Gran, and Andy Wingo. * libguile/hash.c: * libguile/hash.h: * libguile/strings.c: * libguile/strings.h: * libguile/symbols.c: * libguile/symbols.h: Use uintptr_t to store hashes. --- libguile/hash.c | 68 +++++++++++++++++++++++----------------------- libguile/hash.h | 22 +++++++-------- libguile/strings.c | 2 +- libguile/strings.h | 2 +- libguile/symbols.c | 24 ++++++++-------- libguile/symbols.h | 6 ++-- 6 files changed, 62 insertions(+), 62 deletions(-) diff --git a/libguile/hash.c b/libguile/hash.c index a038a11bf..a6d327da5 100644 --- a/libguile/hash.c +++ b/libguile/hash.c @@ -112,25 +112,25 @@ extern double floor(); the hash on a 64-bit system are equal to the hash on a 32-bit \ system. The low 32 bits just add more entropy. */ \ if (sizeof (ret) == 8) \ - ret = (((unsigned long) c) << 32) | b; \ + ret = (((uintptr_t) c) << 32) | b; \ else \ ret = c; \ } while (0) -static unsigned long +static uintptr_t narrow_string_hash (const uint8_t *str, size_t len) { - unsigned long ret; + uintptr_t ret; JENKINS_LOOKUP3_HASHWORD2 (str, len, ret); ret >>= 2; /* Ensure that it fits in a fixnum. */ return ret; } -static unsigned long +static uintptr_t wide_string_hash (const scm_t_wchar *str, size_t len) { - unsigned long ret; + uintptr_t ret; JENKINS_LOOKUP3_HASHWORD2 (str, len, ret); ret >>= 2; /* Ensure that it fits in a fixnum. */ return ret; @@ -138,7 +138,7 @@ wide_string_hash (const scm_t_wchar *str, size_t len) /* If you change this to a different hash, also update (language cps guile-vm). */ -unsigned long +uintptr_t scm_i_string_hash (SCM str) { size_t len = scm_i_string_length (str); @@ -150,13 +150,13 @@ scm_i_string_hash (SCM str) return wide_string_hash (scm_i_string_wide_chars (str), len); } -unsigned long +uintptr_t scm_i_locale_string_hash (const char *str, size_t len) { return scm_i_string_hash (scm_from_locale_stringn (str, len)); } -unsigned long +uintptr_t scm_i_latin1_string_hash (const char *str, size_t len) { if (len == (size_t) -1) @@ -166,11 +166,11 @@ scm_i_latin1_string_hash (const char *str, size_t len) } /* A tricky optimization, but probably worth it. */ -unsigned long +uintptr_t scm_i_utf8_string_hash (const char *str, size_t len) { const uint8_t *end, *ustr = (const uint8_t *) str; - unsigned long ret; + uintptr_t ret; /* The length of the string in characters. This name corresponds to Jenkins' original name. */ @@ -221,8 +221,8 @@ scm_i_utf8_string_hash (const char *str, size_t len) final (a, b, c); - if (sizeof (unsigned long) == 8) - ret = (((unsigned long) c) << 32) | b; + if (sizeof (uintptr_t) == 8) + ret = (((uintptr_t) c) << 32) | b; else ret = c; @@ -230,16 +230,16 @@ scm_i_utf8_string_hash (const char *str, size_t len) return ret; } -static unsigned long scm_raw_ihashq (scm_t_bits key); -static unsigned long scm_raw_ihash (SCM obj, size_t depth); +static uintptr_t scm_raw_ihashq (scm_t_bits key); +static uintptr_t scm_raw_ihash (SCM obj, size_t depth); /* Return the hash of struct OBJ. Traverse OBJ's fields to compute the result, unless DEPTH is zero. Assumes that OBJ is a struct. */ -static unsigned long +static uintptr_t scm_i_struct_hash (SCM obj, size_t depth) { size_t struct_size, field_num; - unsigned long hash; + uintptr_t hash; struct_size = SCM_STRUCT_SIZE (obj); @@ -259,7 +259,7 @@ scm_i_struct_hash (SCM obj, size_t depth) /* Thomas Wang's integer hasher, from http://www.cris.com/~Ttwang/tech/inthash.htm. */ -static unsigned long +static uintptr_t scm_raw_ihashq (scm_t_bits key) { if (sizeof (key) < 8) @@ -285,7 +285,7 @@ scm_raw_ihashq (scm_t_bits key) } /* `depth' is used to limit recursion. */ -static unsigned long +static uintptr_t scm_raw_ihash (SCM obj, size_t depth) { if (SCM_IMP (obj)) @@ -303,7 +303,7 @@ scm_raw_ihash (SCM obj, size_t depth) SCM n = SCM_I_MAKINUM (SCM_MOST_POSITIVE_FIXNUM); if (scm_is_inexact (obj)) obj = scm_inexact_to_exact (obj); - return scm_raw_ihashq (scm_to_ulong (scm_modulo (obj, n))); + return scm_raw_ihashq (scm_to_uintptr_t (scm_modulo (obj, n))); } else return scm_i_string_hash (scm_number_to_string (obj, scm_from_int (10))); @@ -320,7 +320,7 @@ scm_raw_ihash (SCM obj, size_t depth) { size_t len = SCM_SIMPLE_VECTOR_LENGTH (obj); size_t i = depth / 2; - unsigned long h = scm_raw_ihashq (SCM_CELL_WORD_0 (obj)); + uintptr_t h = scm_raw_ihashq (SCM_CELL_WORD_0 (obj)); if (len) while (i--) h ^= scm_raw_ihash (scm_c_vector_ref (obj, h % len), i); @@ -328,7 +328,7 @@ scm_raw_ihash (SCM obj, size_t depth) } case scm_tc7_syntax: { - unsigned long h; + uintptr_t h; h = scm_raw_ihash (scm_syntax_expression (obj), depth); h ^= scm_raw_ihash (scm_syntax_wrap (obj), depth); h ^= scm_raw_ihash (scm_syntax_module (obj), depth); @@ -367,8 +367,8 @@ scm_raw_ihash (SCM obj, size_t depth) \f -unsigned long -scm_ihashq (SCM obj, unsigned long n) +uintptr_t +scm_ihashq (SCM obj, uintptr_t n) { return scm_raw_ihashq (SCM_UNPACK (obj)) % n; } @@ -388,8 +388,8 @@ SCM_DEFINE (scm_hashq, "hashq", 2, 0, 0, "different values, since @code{foo} will be garbage collected.") #define FUNC_NAME s_scm_hashq { - unsigned long sz = scm_to_unsigned_integer (size, 1, ULONG_MAX); - return scm_from_ulong (scm_ihashq (key, sz)); + uintptr_t sz = scm_to_unsigned_integer (size, 1, UINTPTR_MAX); + return scm_from_unsigned_integer (scm_ihashq (key, sz)); } #undef FUNC_NAME @@ -397,8 +397,8 @@ SCM_DEFINE (scm_hashq, "hashq", 2, 0, 0, \f -unsigned long -scm_ihashv (SCM obj, unsigned long n) +uintptr_t +scm_ihashv (SCM obj, uintptr_t n) { if (SCM_NUMP(obj)) return scm_raw_ihash (obj, 10) % n; @@ -421,8 +421,8 @@ SCM_DEFINE (scm_hashv, "hashv", 2, 0, 0, "different values, since @code{foo} will be garbage collected.") #define FUNC_NAME s_scm_hashv { - unsigned long sz = scm_to_unsigned_integer (size, 1, ULONG_MAX); - return scm_from_ulong (scm_ihashv (key, sz)); + uintptr_t sz = scm_to_unsigned_integer (size, 1, UINTPTR_MAX); + return scm_from_unsigned_integer (scm_ihashv (key, sz)); } #undef FUNC_NAME @@ -430,10 +430,10 @@ SCM_DEFINE (scm_hashv, "hashv", 2, 0, 0, \f -unsigned long -scm_ihash (SCM obj, unsigned long n) +uintptr_t +scm_ihash (SCM obj, uintptr_t n) { - return (unsigned long) scm_raw_ihash (obj, 10) % n; + return scm_raw_ihash (obj, 10) % n; } SCM_DEFINE (scm_hash, "hash", 2, 0, 0, @@ -444,8 +444,8 @@ SCM_DEFINE (scm_hash, "hash", 2, 0, 0, "integer in the range 0 to @var{size} - 1.") #define FUNC_NAME s_scm_hash { - unsigned long sz = scm_to_unsigned_integer (size, 1, ULONG_MAX); - return scm_from_ulong (scm_ihash (key, sz)); + uintptr_t sz = scm_to_unsigned_integer (size, 1, UINTPTR_MAX); + return scm_from_unsigned_integer (scm_ihash (key, sz)); } #undef FUNC_NAME diff --git a/libguile/hash.h b/libguile/hash.h index 0e82b4afc..580d2ce93 100644 --- a/libguile/hash.h +++ b/libguile/hash.h @@ -26,19 +26,19 @@ \f -SCM_INTERNAL unsigned long scm_i_locale_string_hash (const char *str, - size_t len); -SCM_INTERNAL unsigned long scm_i_latin1_string_hash (const char *str, - size_t len); -SCM_INTERNAL unsigned long scm_i_utf8_string_hash (const char *str, - size_t len); - -SCM_INTERNAL unsigned long scm_i_string_hash (SCM str); -SCM_API unsigned long scm_ihashq (SCM obj, unsigned long n); +SCM_INTERNAL uintptr_t scm_i_locale_string_hash (const char *str, + size_t len); +SCM_INTERNAL uintptr_t scm_i_latin1_string_hash (const char *str, + size_t len); +SCM_INTERNAL uintptr_t scm_i_utf8_string_hash (const char *str, + size_t len); + +SCM_INTERNAL uintptr_t scm_i_string_hash (SCM str); +SCM_API uintptr_t scm_ihashq (SCM obj, uintptr_t n); SCM_API SCM scm_hashq (SCM obj, SCM n); -SCM_API unsigned long scm_ihashv (SCM obj, unsigned long n); +SCM_API uintptr_t scm_ihashv (SCM obj, uintptr_t n); SCM_API SCM scm_hashv (SCM obj, SCM n); -SCM_API unsigned long scm_ihash (SCM obj, unsigned long n); +SCM_API uintptr_t scm_ihash (SCM obj, uintptr_t n); SCM_API SCM scm_hash (SCM obj, SCM n); SCM_INTERNAL void scm_init_hash (void); diff --git a/libguile/strings.c b/libguile/strings.c index 5eebb3300..572c554c3 100644 --- a/libguile/strings.c +++ b/libguile/strings.c @@ -760,7 +760,7 @@ scm_i_string_set_x (SCM str, size_t p, scm_t_wchar chr) #define SYMBOL_STRINGBUF SCM_CELL_OBJECT_1 SCM -scm_i_make_symbol (SCM name, scm_t_bits flags, unsigned long hash) +scm_i_make_symbol (SCM name, scm_t_bits flags, uintptr_t hash) { SCM buf, symbol; size_t start, length = STRING_LENGTH (name); diff --git a/libguile/strings.h b/libguile/strings.h index f28ef3246..aa6a601be 100644 --- a/libguile/strings.h +++ b/libguile/strings.h @@ -250,7 +250,7 @@ SCM_INTERNAL void scm_i_string_set_x (SCM str, size_t p, scm_t_wchar chr); /* internal functions related to symbols. */ SCM_INTERNAL SCM scm_i_make_symbol (SCM name, scm_t_bits flags, - unsigned long hash); + uintptr_t hash); SCM_INTERNAL const char *scm_i_symbol_chars (SCM sym); SCM_INTERNAL const scm_t_wchar *scm_i_symbol_wide_chars (SCM sym); SCM_INTERNAL size_t scm_i_symbol_length (SCM sym); diff --git a/libguile/symbols.c b/libguile/symbols.c index 292941e9d..b3ddab67d 100644 --- a/libguile/symbols.c +++ b/libguile/symbols.c @@ -71,8 +71,8 @@ SCM_DEFINE (scm_sys_symbols, "%symbols", 0, 0, 0, /* {Symbols} */ -unsigned long -scm_i_hash_symbol (SCM obj, unsigned long n, void *closure) +uintptr_t +scm_i_hash_symbol (SCM obj, uintptr_t n, void *closure) { return scm_i_symbol_hash (obj) % n; } @@ -80,7 +80,7 @@ scm_i_hash_symbol (SCM obj, unsigned long n, void *closure) struct string_lookup_data { SCM string; - unsigned long string_hash; + uintptr_t string_hash; }; static int @@ -102,7 +102,7 @@ string_lookup_predicate_fn (SCM sym, void *closure) } static SCM -lookup_interned_symbol (SCM name, unsigned long raw_hash) +lookup_interned_symbol (SCM name, uintptr_t raw_hash) { struct string_lookup_data data; @@ -118,7 +118,7 @@ struct latin1_lookup_data { const char *str; size_t len; - unsigned long string_hash; + uintptr_t string_hash; }; static int @@ -134,7 +134,7 @@ latin1_lookup_predicate_fn (SCM sym, void *closure) static SCM lookup_interned_latin1_symbol (const char *str, size_t len, - unsigned long raw_hash) + uintptr_t raw_hash) { struct latin1_lookup_data data; @@ -151,7 +151,7 @@ struct utf8_lookup_data { const char *str; size_t len; - unsigned long string_hash; + uintptr_t string_hash; }; static int @@ -201,7 +201,7 @@ utf8_lookup_predicate_fn (SCM sym, void *closure) static SCM lookup_interned_utf8_symbol (const char *str, size_t len, - unsigned long raw_hash) + uintptr_t raw_hash) { struct utf8_lookup_data data; @@ -239,7 +239,7 @@ static SCM scm_i_str2symbol (SCM str) { SCM symbol; - unsigned long raw_hash = scm_i_string_hash (str); + uintptr_t raw_hash = scm_i_string_hash (str); symbol = lookup_interned_symbol (str, raw_hash); if (scm_is_true (symbol)) @@ -261,7 +261,7 @@ scm_i_str2symbol (SCM str) static SCM scm_i_str2uninterned_symbol (SCM str) { - unsigned long raw_hash = scm_i_string_hash (str); + uintptr_t raw_hash = scm_i_string_hash (str); return scm_i_make_symbol (str, SCM_I_F_SYMBOL_UNINTERNED, raw_hash); } @@ -416,7 +416,7 @@ scm_from_latin1_symbol (const char *sym) SCM scm_from_latin1_symboln (const char *sym, size_t len) { - unsigned long hash; + uintptr_t hash; SCM ret; if (len == (size_t) -1) @@ -442,7 +442,7 @@ scm_from_utf8_symbol (const char *sym) SCM scm_from_utf8_symboln (const char *sym, size_t len) { - unsigned long hash; + uintptr_t hash; SCM ret; if (len == (size_t) -1) diff --git a/libguile/symbols.h b/libguile/symbols.h index e8bc3346f..f541f5126 100644 --- a/libguile/symbols.h +++ b/libguile/symbols.h @@ -31,7 +31,7 @@ \f #define scm_is_symbol(x) (SCM_HAS_TYP7 (x, scm_tc7_symbol)) -#define scm_i_symbol_hash(x) ((unsigned long) SCM_CELL_WORD_2 (x)) +#define scm_i_symbol_hash(x) ((uintptr_t) SCM_CELL_WORD_2 (x)) #define scm_i_symbol_is_interned(x) \ (!(SCM_CELL_WORD_0 (x) & SCM_I_F_SYMBOL_UNINTERNED)) @@ -122,8 +122,8 @@ SCM_API SCM scm_take_utf8_symboln (char *sym, size_t len); /* internal functions. */ -SCM_INTERNAL unsigned long scm_i_hash_symbol (SCM obj, unsigned long n, - void *closure); +SCM_INTERNAL uintptr_t scm_i_hash_symbol (SCM obj, uintptr_t n, + void *closure); SCM_INTERNAL void scm_symbols_prehistory (void); SCM_INTERNAL void scm_init_symbols (void); -- 2.45.2 [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: Guile 64-bit Windows support, redux 2024-07-14 18:30 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library @ 2024-07-14 23:10 ` Dr. Arne Babenhauserheide 0 siblings, 0 replies; 23+ messages in thread From: Dr. Arne Babenhauserheide @ 2024-07-14 23:10 UTC (permalink / raw) To: Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library Cc: Ludovic Courtès, Andy Wingo, Jonas Hahnfeld, Michael Käppler [-- Attachment #1: Type: text/plain, Size: 1531 bytes --] Jonas Hahnfeld via "Developers list for Guile, the GNU extensibility library" <guile-devel@gnu.org> writes: > On Wed, 2024-03-20 at 21:28 +0100, Jonas Hahnfeld wrote: >> I'm also explicitly CC'ing Andy and Ludo - we really need a statement >> by a maintainer whether this can land. From my point of view, it's a >> clear improvement in terms of supported platforms, plus tested by >> LilyPond since some time now which is probably one of the bigger >> "customers". > > Another ping on this topic... … > patches downstream in LilyPond, and recently Michael Käppler (CC'ed) > investigated and provided fixes for some more problems with 32-bit long > on Windows that would ideally find their way upstream. > including the patch to store hashes in uintptr_t, as also done by Jan > Nieuwenhuizen, Mike Gran, and Andy Wingo in the wip-mingw branch, which I do not have enough background to judge the other patches, but this one looks safe and good to me. > [5. text/x-patch; 0004-Store-hashes-as-uintptr_t.patch]... If no one sees a problem with that, I can merge, test, and push it, but I’m no maintainer, so I still need a “looks good to me, feel free to push” from a proper maintainer. I can also check whether it makes a performance difference on GNU/Linux (though I doubt that). And in general: almost everything that makes the life easier for Lilypond devs is a win for Guile, too. Best wishes, Arne -- Unpolitisch sein heißt politisch sein, ohne es zu merken. draketo.de [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 1125 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2024-07-14 23:10 UTC | newest] Thread overview: 23+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <1629803116.370682.1686084646758.ref@mail.yahoo.com> 2023-06-06 20:50 ` Guile 64-bit Windows support, redux Mike Gran 2023-06-06 20:56 ` Jean Abou Samra 2023-06-06 21:10 ` [EXT] " Thompson, David 2023-06-06 21:58 ` Mike Gran 2023-06-08 19:50 ` Maxime Devos 2023-06-08 20:46 ` Mike Gran 2023-06-09 11:01 ` Maxime Devos 2023-06-22 13:36 ` Mike Gran 2023-10-29 21:34 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 2023-11-28 21:04 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 2023-11-28 22:04 ` Dr. Arne Babenhauserheide 2024-01-04 10:40 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 2024-02-06 6:44 ` Dr. Arne Babenhauserheide 2024-02-07 14:19 ` Thompson, David 2024-02-07 20:19 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 2024-02-07 20:23 ` Thompson, David 2024-02-07 20:29 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 2024-03-20 20:28 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 2024-03-20 20:40 ` Thompson, David 2024-03-23 15:09 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 2024-03-29 17:20 ` Thompson, David 2024-07-14 18:30 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library 2024-07-14 23:10 ` Dr. Arne Babenhauserheide
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).