Fix definition of some constants when sizeof(mp_limb_t)==4

[wegank]

Apparently, 1UL << 56 is UB when sizeof(long)==4 sizeof(mp_limb_t)==4.

[vneiger]

I would have expected sizeof(long)==4 to be a situation where FLINT sets

define UWORD(xx) (xx##ULL)

and I guess 1ULL << 56 is not UB. Do I expect/guess wrong, then?

See
https://github.com/flintlib/flint/blob/a23a8614dcd0e2838630eaa786a932139a82415f/acinclude.m4#L384
and
https://github.com/flintlib/flint/blob/a23a8614dcd0e2838630eaa786a932139a82415f/src/flint.h.in#L166

[wegank]

A limb is usually long (so sizeof(mp_limb_t) == 4 on 32-bit Linux and 32/64-bit Windows), not long long. So according to configure.ac, FLINT_LONG_LONG is not defined.

If we lived in a world where sizeof(mp_limb_t) == 8 on every 64-bit OS, the port to Windows would have been much simpler...

[vneiger]

A limb is usually long (so sizeof(mp_limb_t) == 4 on 32-bit Linux and 32/64-bit Windows), not long long.

Let's leave out 32-bit things because the piece of code we are discussing assumes a 64-bit context anyway. This is probably not explicitly specified in this file, but it should be if we aim to support 32-bit contexts: these dot products routines via splitting work precisely by accumulating things in 64 bit words to delay modular reductions.

If 64 bits. This is specifically about GMP's mp_limb_t here. As far as I can understand (e.g. from here, https://gmplib.org/manual/ABI-and-ISA and from GMP's configure), building GMP on 64-bit Windows will use ABI=64 by default. And then _LONG_LONG_LIMB is defined to 1, sizeof(mp_limb_t) is 8, and FLINT uses 1ULL. But it's difficult to be certain without actually testing. Did you encounter an actual UB on a machine of yours, on Windows? If one chooses ABI=32, then FLINT_BITS will be set to 32 and I would say that we fall back to the case where this family of dot product implementations should not be used (as is the case at https://github.com/flintlib/flint/blob/a23a8614dcd0e2838630eaa786a932139a82415f/src/nmod_vec/dot.c#L107 ).

If you did encounter a UB mention on UWORD(1) << 56 with a GMP build with ABI=64, that could mean that the issue should be raised at the level of FLINT itself. And that's why I'm a bit picky here, I'd like to minimize how much this set of vectorized functions deviates from what is already or may soon be in FLINT.

[wegank]

I opened this PR for improving 32-bit support (as the branch name suggests), not for supporting 64-bit Windows. That will be addressed in the other PR if I have the time and energy.

I acknowledge my mistake though: on Windows, it was mp_bitcnt_t that has size 4 which makes the porting harder, not mp_limb_t; the latter has size 8. Therefore, on Windows, changing from UWORD to UINT64_C is unnecessary. Still, I think the UL -> ULL part is relevant.

[vneiger]

I opened this PR for improving 32-bit support (as the branch name suggests), not for supporting 64-bit Windows. That will be addressed in the other PR if I have the time and energy.

Ah, I see, so my "let's leave out 32-bit things" was far from your initial idea, sorry for not getting that. Let's have a discussion about this soon, then, since I'm interested in that as well (this probably requires more changes and is related to work that I have in progress around these functions).

[wegank]

This PR now allows 55/59 tests to pass under i686-linux. Combined with the last commit from #214, all tests pass.

[mohabsafey]

Maybe merging this PR with #292 is relevant. Otherwise, I am fine with both

[vneiger]

The modifications here are fine for me. I have a small doubt about the UINT64_C modifications (I guess the dot product approach itself should be modified altogether, to remain close to the same part of FLINT). But these changes are harmless so can be merged, and I will think about with a better view/focus when I add more functions for matrix-matrix products soon.