Azure Pipelines CI + Windows fixes

[Kha]

No description provided.

[leodemoura]

I am assuming is_size_t() must hold here. Then, we have

is_nonneg() && mpz_size(m_val) * sizeof(mp_limb_t) <= sizeof(size_t);

So, the code above seems incorrect if, for example, we have

is_nonneg() == true
mpz_size(m_val) == 2
sizeof(mp_limb_t) == 4
sizeof(size_t) == 8

[leodemoura]

It seems the code above is only correct when sizeof(mp_limb_t) == sizeof(size_t)

[Kha]

Your example does not seem to fulfill the static_assert. However, I see that I did screw up the ordering in the message.

[leodemoura]

My point is that the conditions I listed satisfy is_size_t.
It is not acceptable for get_size_t to fail if is_size_t is satisfied.

[Kha]

But the function cannot fail in this case - the code would refuse to compile in the first place because of the static_assert. Yes, it would be nicer to have higher-level code that works on all imaginable platforms, but GMP only gives us unsigned long accessors. So either we add a messy ifdef for sizeof(unsigned long) < sizeof(size_t) platforms with some shifts generating suboptimal code, or we use this one direct access that CI tells us is safe on all platforms we're interested in. It is possible that the definition of mp_limb_t will change in the future, but I would argue the chance of this being incredibly low - and even then we will immediately know and can fix that line.

[leodemoura]

But the function cannot fail in this case - the code would refuse to compile in the first place because of the static_assert

Fail to compile is bad in this case since the current static_assert does not make sense. One would get the failure and then would find a mismatch between is_size_t and get_size_t. This must be fixed.

[leodemoura]

Yes, it would be nicer to have higher-level code that works on all imaginable platforms, but GMP only gives us unsigned long accessors. So either we add a messy ifdef for sizeof(unsigned long) < sizeof(size_t) platforms with some shifts generating suboptimal code

I am confused now. I thought you started to make these changes because on Windows sizeof(unsigned long) < sizeof(size_t). So, as far as I can tell, this is already an issue. Or, is sizeof(mp_limb_t) == sizeof(size_t) on Windows?

[Kha]

I'm not sure if you were referring to this, but I now see one potential in is_size_t on a hypothetical platform: if sizeof(size_t) < sizeof(mp_limb_t), it would always return false for any positive value. So, here are the solutions I can see right now:

• I assume sizeof(mp_limb_t) == sizeof(size_t) in both functions and simplify is_size_t to is_nonneg() && mpz_size(m_val) <= 1.
• I completely avoid mp_limb_t and reimplement both functions with generic but ugly and suboptimal code.

[leodemoura]

My problem is that the implementation of is_size_t does not match the static_assert at get_size_t.

I assume sizeof(mp_limb_t) == sizeof(size_t) in both functions and simplify is_size_t to is_nonneg() && mpz_size(m_val) <= 1.

Yes, this is a reasonable solution, and we add a static_assert to both functions.

BTW, does static_cast<size_t>(mpz_getlimbn(m_val, 0)) behave correctly when mpz_size(m_val) == 0?

[leodemoura]

Why didn't you name it usize_to_nat?

[leodemoura]

The new and old code seems to be suboptimal here. unsigned_to_nat will perform an unnecessary test. Well, perhaps the compiler will be smart enough to remove it after inlining.

[Kha]

If both are inlined into the same function, the branch will definitely be optimized away. Well, we could get unlucky and have LLVM inline only the outer function, but in that case the overall overhead from that is probably relatively large in general.