r316046 - Basic: fix __{,U}INTPTR_TYPE__ on ARM

[Saleem Abdulrasool via cfe-commits]

On Fri, Oct 20, 2017 at 11:56 AM, Friedman, Eli <efriedma at codeaurora.org>
wrote:

> On 10/17/2017 5:00 PM, Saleem Abdulrasool via cfe-commits wrote:
>
>> Author: compnerd
>> Date: Tue Oct 17 17:00:50 2017
>> New Revision: 316046
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=316046&view=rev
>> Log:
>> Basic: fix __{,U}INTPTR_TYPE__ on ARM
>>
>> Darwin and OpenBSD are the only platforms which use `long int` for
>> `__INTPTR_TYPE__`.  The other platforms use `int` in 32-bit, and `long
>> int` on 64-bit (except for VMS and Windows which are LLP64).  Adjust the
>> type definitions to match the platform definitions.  We now generate the
>> same definition as GCC on all the targets.
>>
>> Modified:
>>      cfe/trunk/lib/Basic/Targets/ARM.cpp
>>      cfe/trunk/test/Preprocessor/init.c
>>      cfe/trunk/test/Preprocessor/stdint.c
>>
>> Modified: cfe/trunk/lib/Basic/Targets/ARM.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targ
>> ets/ARM.cpp?rev=316046&r1=316045&r2=316046&view=diff
>> ============================================================
>> ==================
>> --- cfe/trunk/lib/Basic/Targets/ARM.cpp (original)
>> +++ cfe/trunk/lib/Basic/Targets/ARM.cpp Tue Oct 17 17:00:50 2017
>> @@ -236,6 +236,10 @@ ARMTargetInfo::ARMTargetInfo(const llvm:
>>       break;
>>     }
>>   +  IntPtrType = (Triple.isOSDarwin() || Triple.getOS() ==
>> llvm::Triple::OpenBSD)
>> +                   ? SignedLong
>> +                   : SignedInt;
>> +
>>     // Cache arch related info.
>>     setArchInfo();
>>   @@ -923,7 +927,6 @@ WindowsARMTargetInfo::WindowsARMTargetIn
>>                                              const TargetOptions &Opts)
>>       : WindowsTargetInfo<ARMleTargetInfo>(Triple, Opts), Triple(Triple)
>> {
>>     SizeType = UnsignedInt;
>> -  IntPtrType = SignedInt;
>>   }
>>
>>
>
> Generally, PtrDiffType, IntPtrType, and SizeType are all the same
> (ignoring signedness).  Please change the code to set all of these
> together.  With the code scattered like this, it isn't obvious your changes
> are consistent.  (Actually, I'm pretty sure they aren't consistent.)
>

Yeah, they aren't consistent because AAPCS vs APCS makes things
complicated.  In fact, at least on Darwin we have this beautiful thing:

$ clang -target armv7-apple-ios -mthumb -x c -E /dev/null -dM -o - | grep
-e __SIZE_TYPE__ -e __PTRDIFF_TYPE__ -e __INTPTR_TYPE__
#define __INTPTR_TYPE__ long int
#define __PTRDIFF_TYPE__ int
#define __SIZE_TYPE__ long unsigned int

Seems that we have more ABI inconsistencies to flush out.

$ armv7-unknown-linux-gnueabi-cc -x c -E /dev/null -dM -o - | grep -e
__SIZE_TYPE__ -e __PTRDIFF_TYPE__ -e __INTPTR_TYPE__
#define __SIZE_TYPE__ unsigned int
#define __INTPTR_TYPE__ int
#define __PTRDIFF_TYPE__ int

$ armv7-unknown-linux-gnueabi-cc -mapcs -x c -E /dev/null -dM -o - | grep
-e __SIZE_TYPE__ -e __PTRDIFF_TYPE__ -e __INTPTR_TYPE__
#define __SIZE_TYPE__ unsigned int
#define __INTPTR_TYPE__ int
#define __PTRDIFF_TYPE__ int

$ clang-5.0 -target armv7-unknown-linux-gnu -x c -E /dev/null -dM -o - |
grep -e __SIZE_TYPE__ -e __PTRDIFF_TYPE__ -e __INTPTR_TYPE__
#define __INTPTR_TYPE__ long int
#define __PTRDIFF_TYPE__ int
#define __SIZE_TYPE__ unsigned int

$ clang-5.0 -target armv7-unknown-linux-gnu -Xclang -target-abi -Xclang
apcs-gnu -x c -E /dev/null -dM -o - | grep -e __SIZE_TYPE__ -e
__PTRDIFF_TYPE__ -e __INTPTR_TYPE__
#define __INTPTR_TYPE__ long int
#define __PTRDIFF_TYPE__ int
#define __SIZE_TYPE__ long unsigned int

$ clang-6.0 -target armv7-unknown-linux-gnu -x c -E /dev/null -dM -o - |
grep -e __SIZE_TYPE__ -e __PTRDIFF_TYPE__ -e __INTPTR_TYPE__
#define __INTPTR_TYPE__ int
#define __PTRDIFF_TYPE__ int
#define __SIZE_TYPE__ unsigned int

$ bin/clang-6.0 -target armv7-unknown-linux-gnu -Xclang -target-abi -Xclang
apcs-gnu -x c -E /dev/null -dM -o - | grep -e __SIZE_TYPE__ -e
__PTRDIFF_TYPE__ -e __INTPTR_TYPE__
#define __INTPTR_TYPE__ int
#define __PTRDIFF_TYPE__ int
#define __SIZE_TYPE__ long unsigned int


> Also, in the future, please don't commit any change which affects ABI
> definitions without pre-commit review; this is a tricky area, even if a
> change seems simple.


I'll send out a review for the structuring and fixing the types.


>
> -Eli
>
> --
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
> Foundation Collaborative Project
>

-- 
Saleem Abdulrasool
compnerd (at) compnerd (dot) org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171025/3f3eb8de/attachment-0001.html>