[PATCH] D40285: [MS] AARCH64 cleanup default WIN macros

[Martell Malone via Phabricator via cfe-commits]

martell added a comment.

In https://reviews.llvm.org/D40285#931248, @mstorsjo wrote:

> I'm a little divided - either we remove both WIN32 and WIN64 from all mingw 
> configurations, or we add the missing WIN32 for x86_64 mingw. Removing would 
> be the strictly correct thing to do, but I'm sure it will break code that 
> used to work before (even though it's wrong to rely on the unprefixed one).


looking at different `windef.h` I see - specifically ReactOS

  #ifndef WIN32
  #define WIN32
  #endif

I don't have MSVC off hand to check if it is there but I do remember MSVC 
projects used to pass `/D "WIN32"` so having them might make sense also.
I'm going to try include them and fold all the code for the different arch's 
for mingw in a moment


Repository:
  rL LLVM

https://reviews.llvm.org/D40285



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40285: [MS] AARCH64 cleanup default WIN macros

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In https://reviews.llvm.org/D40285#931242, @martell wrote:

> The easy one is to get rid of WIN64 because gcc doesn't even do that for 
> mingw.


Yes it does, it behaves just the same as WIN32:

  $ x86_64-w64-mingw32-gcc -E -dM - < /dev/null | grep WIN64
  #define _WIN64 1
  #define __WIN64 1
  #define WIN64 1
  #define __WIN64__ 1



> What are your thoughts here on WIN32, I would prefer to move it.

I'm a little divided - either we remove both WIN32 and WIN64 from all mingw 
configurations, or we add the missing WIN32 for x86_64 mingw. Removing would be 
the strictly correct thing to do, but I'm sure it will break code that used to 
work before (even though it's wrong to rely on the unprefixed one).

> I remember a long time ago a lot of projects moved to _WIN32 when they 
> discovered that it was not supposed to be defined.

Yes, generally projects shouldn't check for the unprefixed defines, but 
unfortunately some do. I just recently ran into such an issue when building 
with the x86_64 configuration, where GCC would have had WIN32 defined but clang 
didn't.


Repository:
  rL LLVM

https://reviews.llvm.org/D40285



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40285: [MS] AARCH64 cleanup default WIN macros

[Martell Malone via Phabricator via cfe-commits]

martell added a comment.

... also WINNT was defined for X86 so I just added that to note it.
what do you suggest we do with that one.


Repository:
  rL LLVM

https://reviews.llvm.org/D40285



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40285: [MS] AARCH64 cleanup default WIN macros

[Martell Malone via Phabricator via cfe-commits]

martell added a comment.

In https://reviews.llvm.org/D40285#931241, @mstorsjo wrote:

> ... so in general I wouldn't mind doing a change like this, but I'd like to 
> make it consistent on ARM, i386 and x86_64 at the same time in that case, 
> instead of just changing aarch64.


Thanks for the quick feedback :)
The easy one is to get rid of WIN64 because gcc doesn't even do that for mingw.
What are your thoughts here on WIN32, I would prefer to move it.
I remember a long time ago a lot of projects moved to _WIN32 when they 
discovered that it was not supposed to be defined.
I updated the diff to reflect removing WIN64


Repository:
  rL LLVM

https://reviews.llvm.org/D40285



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40285: [MS] AARCH64 cleanup default WIN macros

[Martell Malone via Phabricator via cfe-commits]

martell updated this revision to Diff 123728.

Repository:
  rL LLVM

https://reviews.llvm.org/D40285

Files:
  lib/Basic/Targets/AArch64.cpp
  lib/Basic/Targets/X86.h
  test/Preprocessor/predefined-macros.c


Index: test/Preprocessor/predefined-macros.c
===
--- test/Preprocessor/predefined-macros.c
+++ test/Preprocessor/predefined-macros.c
@@ -200,12 +200,20 @@
 // CHECK-ARM64-WIN: #define _WIN32 1
 // CHECK-ARM64-WIN: #define _WIN64 1
 
+// RUN: %clang_cc1 -triple x86_64-windows-gnu %s -E -dM -o - \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-AMD64-MINGW
+
+// CHECK-AMD64-MINGW: #define WIN32 1
+// CHECK-AMD64-MINGW: #define WINNT 1
+// CHECK-AMD64-MINGW: #define _WIN32 1
+// CHECK-AMD64-MINGW: #define _WIN64 1
+
 // RUN: %clang_cc1 -triple aarch64-windows-gnu %s -E -dM -o - \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-ARM64-MINGW
 
 // CHECK-ARM64-MINGW-NOT: #define _M_ARM64 1
 // CHECK-ARM64-MINGW: #define WIN32 1
-// CHECK-ARM64-MINGW: #define WIN64 1
+// CHECK-ARM64-MINGW: #define WINNT 1
 // CHECK-ARM64-MINGW: #define _WIN32 1
 // CHECK-ARM64-MINGW: #define _WIN64 1
 // CHECK-ARM64-MINGW: #define __aarch64__ 1
Index: lib/Basic/Targets/X86.h
===
--- lib/Basic/Targets/X86.h
+++ lib/Basic/Targets/X86.h
@@ -726,7 +726,6 @@
   void getTargetDefines(const LangOptions ,
 MacroBuilder ) const override {
 WindowsX86_64TargetInfo::getTargetDefines(Opts, Builder);
-DefineStd(Builder, "WIN64", Opts);
 Builder.defineMacro("__MINGW64__");
 addMinGWDefines(Opts, Builder);
 
Index: lib/Basic/Targets/AArch64.cpp
===
--- lib/Basic/Targets/AArch64.cpp
+++ lib/Basic/Targets/AArch64.cpp
@@ -474,7 +474,6 @@
 void MicrosoftARM64TargetInfo::getVisualStudioDefines(
 const LangOptions , MacroBuilder ) const {
   WindowsTargetInfo::getVisualStudioDefines(Opts, 
Builder);
-  Builder.defineMacro("_WIN32", "1");
   Builder.defineMacro("_WIN64", "1");
   Builder.defineMacro("_M_ARM64", "1");
 }
@@ -494,10 +493,9 @@
 void MinGWARM64TargetInfo::getTargetDefines(const LangOptions ,
 MacroBuilder ) const {
   WindowsTargetInfo::getTargetDefines(Opts, Builder);
-  Builder.defineMacro("_WIN32", "1");
+  DefineStd(Builder, "WIN32", Opts);
+  DefineStd(Builder, "WINNT", Opts);
   Builder.defineMacro("_WIN64", "1");
-  Builder.defineMacro("WIN32", "1");
-  Builder.defineMacro("WIN64", "1");
   addMinGWDefines(Opts, Builder);
 }
 


Index: test/Preprocessor/predefined-macros.c
===
--- test/Preprocessor/predefined-macros.c
+++ test/Preprocessor/predefined-macros.c
@@ -200,12 +200,20 @@
 // CHECK-ARM64-WIN: #define _WIN32 1
 // CHECK-ARM64-WIN: #define _WIN64 1
 
+// RUN: %clang_cc1 -triple x86_64-windows-gnu %s -E -dM -o - \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-AMD64-MINGW
+
+// CHECK-AMD64-MINGW: #define WIN32 1
+// CHECK-AMD64-MINGW: #define WINNT 1
+// CHECK-AMD64-MINGW: #define _WIN32 1
+// CHECK-AMD64-MINGW: #define _WIN64 1
+
 // RUN: %clang_cc1 -triple aarch64-windows-gnu %s -E -dM -o - \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-ARM64-MINGW
 
 // CHECK-ARM64-MINGW-NOT: #define _M_ARM64 1
 // CHECK-ARM64-MINGW: #define WIN32 1
-// CHECK-ARM64-MINGW: #define WIN64 1
+// CHECK-ARM64-MINGW: #define WINNT 1
 // CHECK-ARM64-MINGW: #define _WIN32 1
 // CHECK-ARM64-MINGW: #define _WIN64 1
 // CHECK-ARM64-MINGW: #define __aarch64__ 1
Index: lib/Basic/Targets/X86.h
===
--- lib/Basic/Targets/X86.h
+++ lib/Basic/Targets/X86.h
@@ -726,7 +726,6 @@
   void getTargetDefines(const LangOptions ,
 MacroBuilder ) const override {
 WindowsX86_64TargetInfo::getTargetDefines(Opts, Builder);
-DefineStd(Builder, "WIN64", Opts);
 Builder.defineMacro("__MINGW64__");
 addMinGWDefines(Opts, Builder);
 
Index: lib/Basic/Targets/AArch64.cpp
===
--- lib/Basic/Targets/AArch64.cpp
+++ lib/Basic/Targets/AArch64.cpp
@@ -474,7 +474,6 @@
 void MicrosoftARM64TargetInfo::getVisualStudioDefines(
 const LangOptions , MacroBuilder ) const {
   WindowsTargetInfo::getVisualStudioDefines(Opts, Builder);
-  Builder.defineMacro("_WIN32", "1");
   Builder.defineMacro("_WIN64", "1");
   Builder.defineMacro("_M_ARM64", "1");
 }
@@ -494,10 +493,9 @@
 void MinGWARM64TargetInfo::getTargetDefines(const LangOptions ,
 MacroBuilder ) const {
   WindowsTargetInfo::getTargetDefines(Opts, Builder);
-  Builder.defineMacro("_WIN32", "1");
+  DefineStd(Builder, "WIN32", Opts);
+  DefineStd(Builder, "WINNT", Opts);
   Builder.defineMacro("_WIN64", "1");

[PATCH] D40285: [MS] AARCH64 cleanup default WIN macros

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

... so in general I wouldn't mind doing a change like this, but I'd like to 
make it consistent on ARM, i386 and x86_64 at the same time in that case, 
instead of just changing aarch64.


Repository:
  rL LLVM

https://reviews.llvm.org/D40285



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40285: [MS] AARCH64 cleanup default WIN macros

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

> WIN32 and WIN64 are not real definitions they are typically defined by the 
> system headers, _WIN32 and _WIN64 are the compiler definitions.

Almost.

In MSVC, WIN32 and WIN64 are never defined by the compiler, neither by system 
headers. Project files created by the IDE often contains them set manually 
though.

GCC on the other hand predefines both `_WIN32` and `WIN32` (and similarly for 
-64), but only when using the GNU standards additions (which are enabled by 
default) `x86_64-w64-mingw32-gcc -E -dM - < /dev/null | grep WIN32` does 
include both, while the unprefixed one vanishes if you add e.g. `-std=c99` (but 
are still included if you set `-std=gnu99`).

clang on the other hand doesn't check the standards version, but provides both 
`WIN32` and `_WIN32`. And for the really inconsistent case, with `clang -target 
x86_64-w64-mingw32 -E -dM - < /dev/null`, you will have `WIN64`, `_WIN64` and 
`_WIN32`, but no unprefixed `WIN32`.


Repository:
  rL LLVM

https://reviews.llvm.org/D40285



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40285: [MS] AARCH64 cleanup default WIN macros

[Martell Malone via Phabricator via cfe-commits]

martell created this revision.
Herald added subscribers: kristof.beyls, javed.absar, rengolin, aemerson.

_WIN32 is already defined in lib/Basic/Targets/OSTargets.h
WIN32 and WIN64 are not real definitions they are typically
defined by the system headers,  _WIN32 and _WIN64 are the
compiler definitions.


Repository:
  rL LLVM

https://reviews.llvm.org/D40285

Files:
  lib/Basic/Targets/AArch64.cpp
  test/Preprocessor/predefined-macros.c


Index: test/Preprocessor/predefined-macros.c
===
--- test/Preprocessor/predefined-macros.c
+++ test/Preprocessor/predefined-macros.c
@@ -204,8 +204,6 @@
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-ARM64-MINGW
 
 // CHECK-ARM64-MINGW-NOT: #define _M_ARM64 1
-// CHECK-ARM64-MINGW: #define WIN32 1
-// CHECK-ARM64-MINGW: #define WIN64 1
 // CHECK-ARM64-MINGW: #define _WIN32 1
 // CHECK-ARM64-MINGW: #define _WIN64 1
 // CHECK-ARM64-MINGW: #define __aarch64__ 1
Index: lib/Basic/Targets/AArch64.cpp
===
--- lib/Basic/Targets/AArch64.cpp
+++ lib/Basic/Targets/AArch64.cpp
@@ -474,7 +474,6 @@
 void MicrosoftARM64TargetInfo::getVisualStudioDefines(
 const LangOptions , MacroBuilder ) const {
   WindowsTargetInfo::getVisualStudioDefines(Opts, 
Builder);
-  Builder.defineMacro("_WIN32", "1");
   Builder.defineMacro("_WIN64", "1");
   Builder.defineMacro("_M_ARM64", "1");
 }
@@ -494,10 +493,7 @@
 void MinGWARM64TargetInfo::getTargetDefines(const LangOptions ,
 MacroBuilder ) const {
   WindowsTargetInfo::getTargetDefines(Opts, Builder);
-  Builder.defineMacro("_WIN32", "1");
   Builder.defineMacro("_WIN64", "1");
-  Builder.defineMacro("WIN32", "1");
-  Builder.defineMacro("WIN64", "1");
   addMinGWDefines(Opts, Builder);
 }
 


Index: test/Preprocessor/predefined-macros.c
===
--- test/Preprocessor/predefined-macros.c
+++ test/Preprocessor/predefined-macros.c
@@ -204,8 +204,6 @@
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-ARM64-MINGW
 
 // CHECK-ARM64-MINGW-NOT: #define _M_ARM64 1
-// CHECK-ARM64-MINGW: #define WIN32 1
-// CHECK-ARM64-MINGW: #define WIN64 1
 // CHECK-ARM64-MINGW: #define _WIN32 1
 // CHECK-ARM64-MINGW: #define _WIN64 1
 // CHECK-ARM64-MINGW: #define __aarch64__ 1
Index: lib/Basic/Targets/AArch64.cpp
===
--- lib/Basic/Targets/AArch64.cpp
+++ lib/Basic/Targets/AArch64.cpp
@@ -474,7 +474,6 @@
 void MicrosoftARM64TargetInfo::getVisualStudioDefines(
 const LangOptions , MacroBuilder ) const {
   WindowsTargetInfo::getVisualStudioDefines(Opts, Builder);
-  Builder.defineMacro("_WIN32", "1");
   Builder.defineMacro("_WIN64", "1");
   Builder.defineMacro("_M_ARM64", "1");
 }
@@ -494,10 +493,7 @@
 void MinGWARM64TargetInfo::getTargetDefines(const LangOptions ,
 MacroBuilder ) const {
   WindowsTargetInfo::getTargetDefines(Opts, Builder);
-  Builder.defineMacro("_WIN32", "1");
   Builder.defineMacro("_WIN64", "1");
-  Builder.defineMacro("WIN32", "1");
-  Builder.defineMacro("WIN64", "1");
   addMinGWDefines(Opts, Builder);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits