|
From: Bart V. A. <bva...@ac...> - 2020-04-27 18:09:51
|
On 2020-04-27 11:04, Martin Storsjö wrote: > 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? If that additional change is made, please add an additional comment that explains that _WIN32 is also defined when targeting 64-bit mode. Thanks, Bart. |