ARM: centralise SizeType, PtrDiffType, and IntPtrType

[compnerd]

Summary

Centralise the definitions of these compiler vended types to aid
inspection to ensure that they are defined similarly. The one case that
stands out is the Darwin case where the types do not match up. This
fixes the API conformance for APCS-GNU as well.

[fhahn]

Could you upload a diff with more context, as suggested in http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface? That would make it easier to review the patch.

[efriedma]

I think you missed a couple other places which still set SizeType and PtrDiffType?

[compnerd]

Catch a couple of missed instances, add more context

[compnerd]

lib/Basic/Targets/ARM.cpp

231

I changed this to:

if (Triple.isOSDarwin() && !Triple.isWatchABI())
  PtrDiffType = SignedInt;

[efriedma]

Are you sure the change to APCS is right? I mean, it looks like it's right if I compare to gcc with -mabi=gnu-apcs, but I'm not sure what, exactly, we're trying to be compatible with, so I'd prefer not to touch it, especially not in a patch with a bunch of changes which are supposed to be NFC.

[compnerd]

Well, without matching that ABI, I think that centralizing the logic isn't any cleaner, since we determine the ABI later. With this, we also match the ABI as GNU defines it, and we can move the logic to the same location. intptr_t on Darwin && !WatchOS has the one incompatibility.

[efriedma]

I guess... fine.

LGTM, assuming we have test coverage for all the different cases.

[compnerd]

Yeah, we have coverage for the various environments in test/Preprocessor/init.c. The one case that I didn't find was the APCS-GNU case, which I added a test for additional coverage.

[compnerd]

SVN r316810

lib/Basic/Targets/ARM.cpp

 using namespace clang::targets;
 
 void ARMTargetInfo::setABIAAPCS() {
   IsAAPCS = true;
 
   DoubleAlign = LongLongAlign = LongDoubleAlign = SuitableAlign = 64;
   const llvm::Triple &T = getTriple();
 
-  // size_t is unsigned long on MachO-derived environments, NetBSD, and
-  // OpenBSD.
-  if (T.isOSBinFormatMachO() || T.getOS() == llvm::Triple::NetBSD ||
-      T.getOS() == llvm::Triple::OpenBSD)
-    SizeType = UnsignedLong;
-  else
-    SizeType = UnsignedInt;
-
   bool IsNetBSD = T.getOS() == llvm::Triple::NetBSD;
   bool IsOpenBSD = T.getOS() == llvm::Triple::OpenBSD;
   if (!T.isOSWindows() && !IsNetBSD && !IsOpenBSD)
     WCharType = UnsignedInt;
 
   UseBitFieldTypeAlignment = true;
 
   ZeroLengthBitfieldBoundary = 0;
void ARMTargetInfo::setABIAPCS(bool IsAAPCS16) {
 
   IsAAPCS = false;
 
   if (IsAAPCS16)
     DoubleAlign = LongLongAlign = LongDoubleAlign = SuitableAlign = 64;
   else
     DoubleAlign = LongLongAlign = LongDoubleAlign = SuitableAlign = 32;
 
-  // size_t is unsigned int on FreeBSD.
-  if (T.getOS() == llvm::Triple::FreeBSD)
-    SizeType = UnsignedInt;
-  else
-    SizeType = UnsignedLong;
-
   WCharType = SignedInt;
 
   // Do not respect the alignment of bit-field types when laying out
   // structures. This corresponds to PCC_BITFIELD_TYPE_MATTERS in gcc.
   UseBitFieldTypeAlignment = false;
 
   /// gcc forces the alignment to 4 bytes, regardless of the type of the
   /// zero length bitfield.  This corresponds to EMPTY_FIELD_BOUNDARY in
default:
     return "";
   }
 }
 
 ARMTargetInfo::ARMTargetInfo(const llvm::Triple &Triple,
                              const TargetOptions &Opts)
     : TargetInfo(Triple), FPMath(FP_Default), IsAAPCS(true), LDREX(0),
       HW_FP(0) {
-
-  switch (getTriple().getOS()) {
-  case llvm::Triple::NetBSD:
-  case llvm::Triple::OpenBSD:
-    PtrDiffType = SignedLong;
-    break;
-  default:
-    PtrDiffType = SignedInt;
-    break;
-  }
-
   bool IsOpenBSD = Triple.getOS() == llvm::Triple::OpenBSD;
   bool IsNetBSD = Triple.getOS() == llvm::Triple::NetBSD;
-  IntPtrType =
+
+  PtrDiffType = IntPtrType =
       (Triple.isOSDarwin() || IsOpenBSD || IsNetBSD) ? SignedLong : SignedInt;
 
+  // FIXME: the isOSBinFormatMachO is a workaround for identifying a Darwin-like
+  // environment where size_t is `unsigned long` rather than `unsigned int`
+  SizeType = (Triple.isOSDarwin() || Triple.isOSBinFormatMachO() || IsOpenBSD ||
+              IsNetBSD)
+                 ? UnsignedLong
+                 : UnsignedInt;
+
+  // ptrdiff_t is inconsistent on Darwin
+  if (Triple.isOSDarwin())
+    PtrDiffType = SignedInt;
+  else if (Triple.isWatchABI())
+    PtrDiffType = SignedLong;
+
   // Cache arch related info.
   setArchInfo();
 
   // {} in inline assembly are neon specifiers, not assembly variant
   // specifiers.
   NoAsmVariants = true;
 
   // FIXME: This duplicates code from the driver that sets the -target-abi
void ARMbeTargetInfo::getTargetDefines(const LangOptions &Opts,
   Builder.defineMacro("__ARMEB__");
   Builder.defineMacro("__ARM_BIG_ENDIAN");
   ARMTargetInfo::getTargetDefines(Opts, Builder);
 }
 
 WindowsARMTargetInfo::WindowsARMTargetInfo(const llvm::Triple &Triple,
                                            const TargetOptions &Opts)
     : WindowsTargetInfo<ARMleTargetInfo>(Triple, Opts), Triple(Triple) {
-  SizeType = UnsignedInt;
 }
 
 void WindowsARMTargetInfo::getVisualStudioDefines(const LangOptions &Opts,
                                                   MacroBuilder &Builder) const {
   WindowsTargetInfo<ARMleTargetInfo>::getVisualStudioDefines(Opts, Builder);
 
   // FIXME: this is invalid for WindowsCE
   Builder.defineMacro("_M_ARM_NT", "1");
DarwinARMTargetInfo::DarwinARMTargetInfo(const llvm::Triple &Triple,
   // FIXME: This should be based off of the target features in
   // ARMleTargetInfo.
   MaxAtomicInlineWidth = 64;
 
   if (Triple.isWatchABI()) {
     // Darwin on iOS uses a variant of the ARM C++ ABI.
     TheCXXABI.set(TargetCXXABI::WatchOS);
 
-    // The 32-bit ABI is silent on what ptrdiff_t should be, but given that
-    // size_t is long, it's a bit weird for it to be int.
-    PtrDiffType = SignedLong;
-
     // BOOL should be a real boolean on the new ABI
     UseSignedCharForObjCBool = false;
   } else
     TheCXXABI.set(TargetCXXABI::iOS);
 }
 
 void DarwinARMTargetInfo::getOSDefines(const LangOptions &Opts,
                                        const llvm::Triple &Triple,

test/Preprocessor/init.c

 // ARM:#define __WCHAR_MAX__ 4294967295U
 // ARM:#define __WCHAR_TYPE__ unsigned int
 // ARM:#define __WCHAR_WIDTH__ 32
 // ARM:#define __WINT_TYPE__ int
 // ARM:#define __WINT_WIDTH__ 32
 // ARM:#define __arm 1
 // ARM:#define __arm__ 1
 
+// RUN: %clang_cc1 -dM -ffreestanding -triple arm-none-none -target-abi apcs-gnu -E /dev/null -o - | FileCheck -match-full-lines -check-prefix ARM-APCS-GNU %s
+// ARM-APCS-GNU: #define __INTPTR_TYPE__ int
+// ARM-APCS-GNU: #define __PTRDIFF_TYPE__ int
+// ARM-APCS-GNU: #define __SIZE_TYPE__ unsigned int
+
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=armeb-none-none < /dev/null | FileCheck -match-full-lines -check-prefix ARM-BE %s
 //
 // ARM-BE-NOT:#define _LP64
 // ARM-BE:#define __APCS_32__ 1
 // ARM-BE:#define __ARMEB__ 1
 // ARM-BE-NOT:#define __ARMEL__ 1
 // ARM-BE:#define __ARM_ARCH_4T__ 1
 // ARM-BE:#define __ARM_BIG_ENDIAN 1