|
From: Martin S. <ma...@ma...> - 2020-04-27 18:04:33
|
On Mon, 27 Apr 2020, Bart Van Assche wrote: > On 2020-04-04 13:35, Martin Storsjö wrote: >> Don't assume that __MINGW32__ implies x86; Windows runs on ARM/ARM64 >> as well, and there are mingw toolchains that target those architectures. >> >> This mirrors how the MSVC part of the same expressions are written, >> as (defined(_WIN32) && defined(_M_IX86)) and >> (defined(_WIN64) && defined(_M_X64)) - not relying on _WIN32/_WIN64 >> or __MINGW32__/__MINGW64__ alone to indicate architecture. >> --- >> include/valgrind.h | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/include/valgrind.h b/include/valgrind.h >> index c8b24a38e..e8195c1ce 100644 >> --- a/include/valgrind.h >> +++ b/include/valgrind.h >> @@ -131,11 +131,11 @@ >> # define PLAT_x86_darwin 1 >> #elif defined(__APPLE__) && defined(__x86_64__) >> # define PLAT_amd64_darwin 1 >> -#elif (defined(__MINGW32__) && !defined(__MINGW64__)) \ >> +#elif (defined(__MINGW32__) && defined(__i386__)) \ >> || defined(__CYGWIN32__) \ >> || (defined(_WIN32) && defined(_M_IX86)) >> # define PLAT_x86_win32 1 >> -#elif defined(__MINGW64__) \ >> +#elif (defined(__MINGW64__) && defined(__x86_64__)) \ >> || (defined(_WIN64) && defined(_M_X64)) >> # define PLAT_amd64_win64 1 >> #elif defined(__linux__) && defined(__i386__) > > How about changing __MINGW64__ into __MINGW32__? I think that will make > this patch easier to read. From commit d406e8725c09: "mingw64 also > defines __MINGW32__". Please also add a comment that the MinGW64 > compiler defines __MINGW32__. That's fine with me as well, but there is the same kind of tautology for the MSVC variant of ifdefs right next to it, "defined(_WIN64) && defined(_M_X64)". Should I keep that as is or do the same kind of cleanup for that as well? // Martin |