boostorg / thread

Boost.org thread module
http://boost.org/libs/thread
199 stars 161 forks source link

Compiling for ARM32/64 on VS2017 broken #233

Closed DjArt closed 5 years ago

DjArt commented 5 years ago

Due assign in interlocked_read.hpp:

        inline void* interlocked_read_acquire(void* volatile* x) BOOST_NOEXCEPT
        {
            void* const res=
#if defined(_M_ARM64)
                __iso_volatile_load64((const volatile __int64*)x);
#else
                __iso_volatile_load32((const volatile __int32*)x);
#endif
            BOOST_THREAD_DETAIL_COMPILER_BARRIER();
            __dmb(0xB); // _ARM_BARRIER_ISH, see armintr.h from MSVC 11 and later
            BOOST_THREAD_DETAIL_COMPILER_BARRIER();
            return res;
        }
viboes commented 5 years ago

Please, could you add the report you are getting?

DjArt commented 5 years ago

@viboes, sorry for the long wait. Error C2440: int can't be converted to void*.

viboes commented 5 years ago

Don't worry.

If the function doesn't returns a void, but an int, why do we want to store it on a void *?

DjArt commented 5 years ago

void* used as platform-unspecified(by size) pointer. So, I think that's better variant for saving original idea of code.

viboes commented 5 years ago

Oh, I see.

So IIUC, iso_volatile_load64 returned always a int64 and iso_volatile_load32 returned always int32.

And this size corresponds to the size of a pointer depending on whether _M_ARM64 is defined or not.

How it is that this worked before, Andrei, what do you think?

DjArt commented 5 years ago

I think, or this code was untested before, or MSVC for ARM has become more strict then before. x86/x86_64 compiler doesn't consider it as mistake.

viboes commented 5 years ago

I believe the code needs a static assert about the size of both types. Could you add it, please?