This project is read-only.
2

Resolved

bug gcc 4.3.2 and 4.4.1 int64_t

description

i try to use multiply on int64_t
 
boost::int64_t a=2;
boost::int64_t b=3;
boost::int64_t c;
SafeMultiply<boost::int64_t,boost::int64_t>(a,b,c);
 
gcc 4.3.2 and 4.4.1 takes compile time error
 
SafeInt3.hpp: In function ‘bool SafeMultiply(T, U, T&) [with T = long int, U = long int]’:
test1.cpp:233: instantiated from here
SafeInt3.hpp:4995: error: incomplete type ‘MultiplicationHelper<long int, long int, 11>’ used in nested name specifier

file attachments

comments

bmb wrote Jan 13, 2010 at 12:43 AM

Hi, I see this problem too. I think the issue stems from 64-bit GCC -- on my platform at least,
sizeof(long) == 8
sizeof(long long) == 8
To the type checker, those types aren't equivalent, even though they have the same size. But, the existing GCC code essentially assumes that long long (a.k.a __int64) is the only 64 bit type, so any Helper specialization that's 64-bit only takes __int64 directly instead of taking a type argument.

I took a stab at fixing this and have a (prototype) patch, it adds template arguments to several Helper definitions so that they can be generated for long and for long long. Also included in this patch are workarounds to eliminate GCC warnings (-Wall is clean for me on gcc 4.2.4). I've only compile-tested so far, and only on GCC.

Does something like this seem workable? If so, I'd be happy to clean it up and submit it. (and separate it into 2 parts if need be)

Thanks,
Brian Bloniarz

wrote Jan 13, 2010 at 12:43 AM

wrote Jan 13, 2010 at 12:49 AM

dcleblanc wrote Jan 13, 2010 at 4:21 AM

Sorry I haven't responded more quickly. The normal use of SafeMultiply is to let the compiler sort out the types by itself, so it would be:

__int64 a, b, c;
a = x;
b = y;
SafeMultiply(a, b, c);

Your analysis is correct - all the internal functions take __int64 directly. I can't see the patch from this (Windows) system, so I'd need to look at the changes. On a Microsoft compiler, the only way to get a 64-bit int is either long long or __int64. I don't think I'd considered long int. With my compiler, I get this:
long int a, b, c;
a = 1;
b = 2;
SafeMultiply(a, b, c);

MultiplicationHelper<long,long,1>::Multiply(const long & t=1, const long & u=2, long & ret=-858993460) Line 1576 C++
But then if I make them long long, I get:
MultiplicationHelper<__int64,__int64,11>::Multiply(const __int64 & t=1, const __int64 & u=2, __int64 & ret=-3689348814741910324) Line 2606 C++
I have no idea what the standard says about these types, and whose compiler is right, at least with respect to long int vs. long long. I'm fully aware that a long can be any length according to the standard. I have to wonder if we can get away with just not specifying the first 2 template types - looks like the code would work properly either way. For example, we have:

template <> class MultiplicationHelper<__int64, __int64, MultiplicationState_Int64Int64 >

And there is exactly one that refers to MultiplicationState_Int64Int64, and I think we could just change it to:

template <typename T, typename U> class MultiplicationHelper<T, U, MultiplicationState_Int64Int64 >

It should still work, and we could add a compile assert that sizeof(T) == 8

Is that what you did?

bmb wrote Jan 13, 2010 at 2:36 PM

That's what I did, though I didn't think to add a compile-time assert :) It took new template arguments to some of the Multiplication, Divison, Addition and Subtraction helpers to make it all work.

Additionally, LargeIntRegMultiply<> was tricky... think about:
template<> class LargeIntRegMultiply< unsigned __int64, unsigned __int64 >
You can't just change this to template<typename T, typename U>, the same way you can for the various Helpers, since those types are used to select which specialization you want. The way I worked around this was by
changing it to:
template<> class LargeIntRegMultiply< unsigned __int64, unsigned __int64 >
{
template < typename T, typename U >
static bool RegMultiply( const T& a, const U& b, T& ret ) throw() ...
}
So the class's template arguments identify the size of the arguments, but you can still call RegMultiply() with unsigned long arguments.

Another alternative would be to leave LargeIntRegMultiply as __int64-only, and convert from the true argument types inside MultiplicationHelper. Now that I think about it, this might be cleaner, maybe a little more invasive change though.

dcleblanc wrote Jan 13, 2010 at 8:19 PM

The RegMultiply stuff needs to be kept to internal calls - for example, on the x64 Microsoft compiler, we can get some gains by using intrinsics that just do 128-bit math, just like we check 32-bit math with 64-bit. So I think it is OK to use the __int64 on stuff that we know is purely internal. Once we have the proper template specialization, then I think we're OK to force things to the __int64 type.

dcleblanc wrote Feb 4, 2010 at 6:10 AM

This is now fixed by planned release 3.0.13p

wrote Feb 4, 2010 at 6:11 AM

wrote Feb 14, 2013 at 2:31 AM

wrote May 16, 2013 at 9:03 AM

wrote May 16, 2013 at 9:03 AM

wrote Jun 14, 2013 at 7:46 AM