[MINGW] normalize WIN32 macros

[martell]

Summary

move _WIN64 and _WIN32 defones to lib/Basic/Targets/OSTargets.h
move WIN32, WIN64 and MINGW64 to addMinGWDefines
remove unnecessary overrides

[mstorsjo]

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.

[mstorsjo]

... 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.

[martell]

In 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

[martell]

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

[mstorsjo]

In 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.

[martell]

In 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

[martell]

[MINGW] normalize WIN32 macros

[mstorsjo]

LGTM

test/Preprocessor/predefined-macros.c

200 ↗

(On Diff #123747)

In general, these kinds of -NOT tests don't work quite as one would want here - if the non-wanted #define _WIN64 would appear before the expected #define _WIN32, it would actually pass this test. Usually the -DAG kinds of checks can be used to avoid that issue. Not sure how well it mixes with -DAG and -DAG-NOT though.

Can we assume that these defines appear in alphabetical order? In that case, we can be pretty sure to catch the issue if they're reordered slightly. This at least clearly documents what is the intended behaviour in any case.

[martell]

reorder test checks

[martell]

@mstorsjo can you confirm this new order looks good to you?

[mstorsjo]

LGTM

cfe/trunk/lib/Basic/Targets.h

 void DefineStd(clang::MacroBuilder &Builder, llvm::StringRef MacroName,
                const clang::LangOptions &Opts);
 
 LLVM_LIBRARY_VISIBILITY
 void defineCPUMacros(clang::MacroBuilder &Builder, llvm::StringRef CPUName,
                      bool Tuning = true);
 
 LLVM_LIBRARY_VISIBILITY
-void addMinGWDefines(const clang::LangOptions &Opts,
+void addMinGWDefines(const llvm::Triple &Triple, const clang::LangOptions &Opts,
                      clang::MacroBuilder &Builder);
 
 LLVM_LIBRARY_VISIBILITY
 void addCygMingDefines(const clang::LangOptions &Opts,
                        clang::MacroBuilder &Builder);
 } // namespace targets
 } // namespace clang
 #endif // LLVM_CLANG_LIB_BASIC_TARGETS_H

cfe/trunk/lib/Basic/Targets.cpp

for (const char *CC : CCs) {
       GCCSpelling += CC;
       GCCSpelling += "__))";
       Builder.defineMacro(Twine("_") + CC, GCCSpelling);
       Builder.defineMacro(Twine("__") + CC, GCCSpelling);
     }
   }
 }
 
-void addMinGWDefines(const LangOptions &Opts, MacroBuilder &Builder) {
+void addMinGWDefines(const llvm::Triple &Triple, const LangOptions &Opts,
+                     MacroBuilder &Builder) {
+  DefineStd(Builder, "WIN32", Opts);
+  DefineStd(Builder, "WINNT", Opts);
+  if (Triple.isArch64Bit()) {
+    DefineStd(Builder, "WIN64", Opts);
+    Builder.defineMacro("__MINGW64__");
+  }
   Builder.defineMacro("__MSVCRT__");
   Builder.defineMacro("__MINGW32__");
   addCygMingDefines(Opts, Builder);
 }
 
 //===----------------------------------------------------------------------===//
 // Driver code
 //===----------------------------------------------------------------------===//

cfe/trunk/lib/Basic/Targets/AArch64.h

void getTargetDefines(const LangOptions &Opts,
                         MacroBuilder &Builder) const override;
 };
 
 // ARM64 MinGW target
 class LLVM_LIBRARY_VISIBILITY MinGWARM64TargetInfo
     : public WindowsARM64TargetInfo {
 public:
   MinGWARM64TargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts);
-
-  void getTargetDefines(const LangOptions &Opts,
-                        MacroBuilder &Builder) const override;
 };
 
 class LLVM_LIBRARY_VISIBILITY AArch64beTargetInfo : public AArch64TargetInfo {
 public:
   AArch64beTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts);
   void getTargetDefines(const LangOptions &Opts,
                         MacroBuilder &Builder) const override;
 

cfe/trunk/lib/Basic/Targets/AArch64.cpp

MicrosoftARM64TargetInfo::MicrosoftARM64TargetInfo(const llvm::Triple &Triple,
                                                    const TargetOptions &Opts)
     : WindowsARM64TargetInfo(Triple, Opts) {
   TheCXXABI.set(TargetCXXABI::Microsoft);
 }
 
 void MicrosoftARM64TargetInfo::getVisualStudioDefines(
     const LangOptions &Opts, MacroBuilder &Builder) const {
   WindowsTargetInfo<AArch64leTargetInfo>::getVisualStudioDefines(Opts, Builder);
-  Builder.defineMacro("_WIN32", "1");
-  Builder.defineMacro("_WIN64", "1");
   Builder.defineMacro("_M_ARM64", "1");
 }
 
 void MicrosoftARM64TargetInfo::getTargetDefines(const LangOptions &Opts,
                                                 MacroBuilder &Builder) const {
   WindowsTargetInfo::getTargetDefines(Opts, Builder);
   getVisualStudioDefines(Opts, Builder);
 }
 
 MinGWARM64TargetInfo::MinGWARM64TargetInfo(const llvm::Triple &Triple,
                                            const TargetOptions &Opts)
     : WindowsARM64TargetInfo(Triple, Opts) {
   TheCXXABI.set(TargetCXXABI::GenericAArch64);
 }
 
-void MinGWARM64TargetInfo::getTargetDefines(const LangOptions &Opts,
-                                            MacroBuilder &Builder) const {
-  WindowsTargetInfo::getTargetDefines(Opts, Builder);
-  Builder.defineMacro("_WIN32", "1");
-  Builder.defineMacro("_WIN64", "1");
-  Builder.defineMacro("WIN32", "1");
-  Builder.defineMacro("WIN64", "1");
-  addMinGWDefines(Opts, Builder);
-}
-
-
 DarwinAArch64TargetInfo::DarwinAArch64TargetInfo(const llvm::Triple &Triple,
                                                  const TargetOptions &Opts)
     : DarwinTargetInfo<AArch64leTargetInfo>(Triple, Opts) {
   Int64Type = SignedLongLong;
   UseSignedCharForObjCBool = false;
 
   LongDoubleWidth = LongDoubleAlign = SuitableAlign = 64;
   LongDoubleFormat = &llvm::APFloat::IEEEdouble();

cfe/trunk/lib/Basic/Targets/ARM.cpp

MinGWARMTargetInfo::MinGWARMTargetInfo(const llvm::Triple &Triple,
                                        const TargetOptions &Opts)
     : WindowsARMTargetInfo(Triple, Opts) {
   TheCXXABI.set(TargetCXXABI::GenericARM);
 }
 
 void MinGWARMTargetInfo::getTargetDefines(const LangOptions &Opts,
                                           MacroBuilder &Builder) const {
   WindowsARMTargetInfo::getTargetDefines(Opts, Builder);
-  DefineStd(Builder, "WIN32", Opts);
-  DefineStd(Builder, "WINNT", Opts);
   Builder.defineMacro("_ARM_");
   Builder.defineMacro("__ARM_DWARF_EH__");
-  addMinGWDefines(Opts, Builder);
 }
 
 CygwinARMTargetInfo::CygwinARMTargetInfo(const llvm::Triple &Triple,
                                          const TargetOptions &Opts)
     : ARMleTargetInfo(Triple, Opts) {
   this->WCharType = TargetInfo::UnsignedShort;
   TLSSupported = false;
   DoubleAlign = LongLongAlign = 64;

cfe/trunk/lib/Basic/Targets/OSTargets.h

 
 // Windows target
 template <typename Target>
 class LLVM_LIBRARY_VISIBILITY WindowsTargetInfo : public OSTargetInfo<Target> {
 protected:
   void getOSDefines(const LangOptions &Opts, const llvm::Triple &Triple,
                     MacroBuilder &Builder) const override {
     Builder.defineMacro("_WIN32");
+    if (Triple.isArch64Bit())
+      Builder.defineMacro("_WIN64");
+    if (Triple.isWindowsGNUEnvironment())
+      addMinGWDefines(Triple, Opts, Builder);
+
   }
   void getVisualStudioDefines(const LangOptions &Opts,
                               MacroBuilder &Builder) const {
     if (Opts.CPlusPlus) {
       if (Opts.RTTIData)
         Builder.defineMacro("_CPPRTTI");
 
       if (Opts.CXXExceptions)

cfe/trunk/lib/Basic/Targets/X86.h

WindowsX86_32TargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
       : WindowsTargetInfo<X86_32TargetInfo>(Triple, Opts) {
     DoubleAlign = LongLongAlign = 64;
     bool IsWinCOFF =
         getTriple().isOSWindows() && getTriple().isOSBinFormatCOFF();
     resetDataLayout(IsWinCOFF
                         ? "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
                         : "e-m:e-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32");
   }
-
-  void getTargetDefines(const LangOptions &Opts,
-                        MacroBuilder &Builder) const override {
-    WindowsTargetInfo<X86_32TargetInfo>::getTargetDefines(Opts, Builder);
-  }
 };
 
 // x86-32 Windows Visual Studio target
 class LLVM_LIBRARY_VISIBILITY MicrosoftX86_32TargetInfo
     : public WindowsX86_32TargetInfo {
 public:
   MicrosoftX86_32TargetInfo(const llvm::Triple &Triple,
                             const TargetOptions &Opts)
public:
   MinGWX86_32TargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
       : WindowsX86_32TargetInfo(Triple, Opts) {
     HasFloat128 = true;
   }
 
   void getTargetDefines(const LangOptions &Opts,
                         MacroBuilder &Builder) const override {
     WindowsX86_32TargetInfo::getTargetDefines(Opts, Builder);
-    DefineStd(Builder, "WIN32", Opts);
-    DefineStd(Builder, "WINNT", Opts);
     Builder.defineMacro("_X86_");
-    addMinGWDefines(Opts, Builder);
   }
 };
 
 // x86-32 Cygwin target
 class LLVM_LIBRARY_VISIBILITY CygwinX86_32TargetInfo : public X86_32TargetInfo {
 public:
   CygwinX86_32TargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
       : X86_32TargetInfo(Triple, Opts) {
WindowsX86_64TargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
     DoubleAlign = LongLongAlign = 64;
     IntMaxType = SignedLongLong;
     Int64Type = SignedLongLong;
     SizeType = UnsignedLongLong;
     PtrDiffType = SignedLongLong;
     IntPtrType = SignedLongLong;
   }
 
-  void getTargetDefines(const LangOptions &Opts,
-                        MacroBuilder &Builder) const override {
-    WindowsTargetInfo<X86_64TargetInfo>::getTargetDefines(Opts, Builder);
-    Builder.defineMacro("_WIN64");
-  }
-
   BuiltinVaListKind getBuiltinVaListKind() const override {
     return TargetInfo::CharPtrBuiltinVaList;
   }
 
   CallingConvCheckResult checkCallingConvention(CallingConv CC) const override {
     switch (CC) {
     case CC_X86StdCall:
     case CC_X86ThisCall:
MinGWX86_64TargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
     LongDoubleWidth = LongDoubleAlign = 128;
     LongDoubleFormat = &llvm::APFloat::x87DoubleExtended();
     HasFloat128 = true;
   }
 
   void getTargetDefines(const LangOptions &Opts,
                         MacroBuilder &Builder) const override {
     WindowsX86_64TargetInfo::getTargetDefines(Opts, Builder);
-    DefineStd(Builder, "WIN64", Opts);
-    Builder.defineMacro("__MINGW64__");
-    addMinGWDefines(Opts, Builder);
 
     // GCC defines this macro when it is using __gxx_personality_seh0.
     if (!Opts.SjLjExceptions)
       Builder.defineMacro("__SEH__");
   }
 };
 
 // x86-64 Cygwin target

cfe/trunk/test/Preprocessor/predefined-macros.c

 // RUN: %clang_cc1 %s -E -dM -o - -x cl \
 // RUN:   | FileCheck %s --check-prefix=MSCOPE
 // MSCOPE:#define __OPENCL_MEMORY_SCOPE_ALL_SVM_DEVICES 3
 // MSCOPE:#define __OPENCL_MEMORY_SCOPE_DEVICE 2
 // MSCOPE:#define __OPENCL_MEMORY_SCOPE_SUB_GROUP 4
 // MSCOPE:#define __OPENCL_MEMORY_SCOPE_WORK_GROUP 1
 // MSCOPE:#define __OPENCL_MEMORY_SCOPE_WORK_ITEM 0
 
+// RUN: %clang_cc1 -triple i386-windows %s -E -dM -o - \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-X86-WIN
+
+// CHECK-X86-WIN-NOT: #define WIN32 1
+// CHECK-X86-WIN-NOT: #define WIN64 1
+// CHECK-X86-WIN-NOT: #define WINNT 1
+// CHECK-X86-WIN: #define _WIN32 1
+// CHECK-X86-WIN-NOT: #define _WIN64 1
+
+// RUN: %clang_cc1 -triple thumbv7-windows %s -E -dM -o - \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-ARM-WIN
+
+// CHECK-ARM-WIN-NOT: #define WIN32 1
+// CHECK-ARM-WIN-NOT: #define WIN64 1
+// CHECK-ARM-WIN-NOT: #define WINNT 1
+// CHECK-ARM-WIN: #define _WIN32 1
+// CHECK-ARM-WIN-NOT: #define _WIN64 1
+
+// RUN: %clang_cc1 -triple x86_64-windows %s -E -dM -o - \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-AMD64-WIN
+
+// CHECK-AMD64-WIN-NOT: #define WIN32 1
+// CHECK-AMD64-WIN-NOT: #define WIN64 1
+// CHECK-AMD64-WIN-NOT: #define WINNT 1
+// CHECK-AMD64-WIN: #define _WIN32 1
+// CHECK-AMD64-WIN: #define _WIN64 1
+
 // RUN: %clang_cc1 -triple aarch64-windows %s -E -dM -o - \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-ARM64-WIN
 
+// CHECK-ARM64-WIN-NOT: #define WIN32 1
+// CHECK-ARM64-WIN-NOT: #define WIN64 1
+// CHECK-ARM64-WIN-NOT: #define WINNT 1
 // CHECK-ARM64-WIN: #define _M_ARM64 1
 // CHECK-ARM64-WIN: #define _WIN32 1
 // CHECK-ARM64-WIN: #define _WIN64 1
 
+// RUN: %clang_cc1 -triple i686-windows-gnu %s -E -dM -o - \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-X86-MINGW
+
+// CHECK-X86-MINGW: #define WIN32 1
+// CHECK-X86-MINGW-NOT: #define WIN64 1
+// CHECK-X86-MINGW: #define WINNT 1
+// CHECK-X86-MINGW: #define _WIN32 1
+// CHECK-X86-MINGW-NOT: #define _WIN64 1
+
+// RUN: %clang_cc1 -triple thumbv7-windows-gnu %s -E -dM -o - \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-ARM-MINGW
+
+// CHECK-ARM-MINGW: #define WIN32 1
+// CHECK-ARM-MINGW-NOT: #define WIN64 1
+// CHECK-ARM-MINGW: #define WINNT 1
+// CHECK-ARM-MINGW: #define _WIN32 1
+// CHECK-ARM-MINGW-NOT: #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 WIN64 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