<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Oct 20, 2017 at 11:56 AM, Friedman, Eli <span dir="ltr"><<a href="mailto:efriedma@codeaurora.org" target="_blank">efriedma@codeaurora.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div class="gmail-HOEnZb"><div class="gmail-h5">On 10/17/2017 5:00 PM, Saleem Abdulrasool via cfe-commits wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
Author: compnerd<br>
Date: Tue Oct 17 17:00:50 2017<br>
New Revision: 316046<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=316046&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=316046&view=rev</a><br>
Log:<br>
Basic: fix __{,U}INTPTR_TYPE__ on ARM<br>
<br>
Darwin and OpenBSD are the only platforms which use `long int` for<br>
`__INTPTR_TYPE__`. The other platforms use `int` in 32-bit, and `long<br>
int` on 64-bit (except for VMS and Windows which are LLP64). Adjust the<br>
type definitions to match the platform definitions. We now generate the<br>
same definition as GCC on all the targets.<br>
<br>
Modified:<br>
cfe/trunk/lib/Basic/Targets/A<wbr>RM.cpp<br>
cfe/trunk/test/Preprocessor/i<wbr>nit.c<br>
cfe/trunk/test/Preprocessor/s<wbr>tdint.c<br>
<br>
Modified: cfe/trunk/lib/Basic/Targets/AR<wbr>M.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/ARM.cpp?rev=316046&r1=316045&r2=316046&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/lib/Basic/Targ<wbr>ets/ARM.cpp?rev=316046&r1=3160<wbr>45&r2=316046&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Basic/Targets/AR<wbr>M.cpp (original)<br>
+++ cfe/trunk/lib/Basic/Targets/AR<wbr>M.cpp Tue Oct 17 17:00:50 2017<br>
@@ -236,6 +236,10 @@ ARMTargetInfo::ARMTargetInfo(c<wbr>onst llvm:<br>
break;<br>
}<br>
+ IntPtrType = (Triple.isOSDarwin() || Triple.getOS() == llvm::Triple::OpenBSD)<br>
+ ? SignedLong<br>
+ : SignedInt;<br>
+<br>
// Cache arch related info.<br>
setArchInfo();<br>
@@ -923,7 +927,6 @@ WindowsARMTargetInfo::WindowsA<wbr>RMTargetIn<br>
const TargetOptions &Opts)<br>
: WindowsTargetInfo<ARMleTargetI<wbr>nfo>(Triple, Opts), Triple(Triple) {<br>
SizeType = UnsignedInt;<br>
- IntPtrType = SignedInt;<br>
}<br>
<br>
</blockquote>
<br></div></div>
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.)<br></blockquote><div><br></div><div>Yeah, they aren't consistent because AAPCS vs APCS makes things complicated. In fact, at least on Darwin we have this beautiful thing:</div><div><br></div><div><div>$ clang -target armv7-apple-ios -mthumb -x c -E /dev/null -dM -o - | grep -e __SIZE_TYPE__ -e __PTRDIFF_TYPE__ -e __INTPTR_TYPE__</div><div>#define __INTPTR_TYPE__ long int</div><div>#define __PTRDIFF_TYPE__ int</div><div>#define __SIZE_TYPE__ long unsigned int</div></div><div><br></div><div>Seems that we have more ABI inconsistencies to flush out.</div><div><br></div><div><div>$ armv7-unknown-linux-gnueabi-cc -x c -E /dev/null -dM -o - | grep -e __SIZE_TYPE__ -e __PTRDIFF_TYPE__ -e __INTPTR_TYPE__</div><div>#define __SIZE_TYPE__ unsigned int</div><div>#define __INTPTR_TYPE__ int</div><div>#define __PTRDIFF_TYPE__ int</div></div><div><br></div><div><div>$ armv7-unknown-linux-gnueabi-cc -mapcs -x c -E /dev/null -dM -o - | grep -e __SIZE_TYPE__ -e __PTRDIFF_TYPE__ -e __INTPTR_TYPE__</div><div>#define __SIZE_TYPE__ unsigned int</div><div>#define __INTPTR_TYPE__ int</div><div>#define __PTRDIFF_TYPE__ int</div></div><div><br></div><div><div>$ 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__</div><div><div>#define __INTPTR_TYPE__ long int</div><div>#define __PTRDIFF_TYPE__ int</div><div>#define __SIZE_TYPE__ unsigned int</div></div></div><div><br></div><div><div>$ 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__</div><div>#define __INTPTR_TYPE__ long int</div><div>#define __PTRDIFF_TYPE__ int</div><div>#define __SIZE_TYPE__ long unsigned int</div></div><div><br></div><div>$ 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__</div><div><div>#define __INTPTR_TYPE__ int</div><div>#define __PTRDIFF_TYPE__ int</div><div>#define __SIZE_TYPE__ unsigned int</div></div><div><br></div><div><div>$ 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__</div><div>#define __INTPTR_TYPE__ int</div><div>#define __PTRDIFF_TYPE__ int</div><div>#define __SIZE_TYPE__ long unsigned int</div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
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.</blockquote><div><br></div><div>I'll send out a review for the structuring and fixing the types.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><span class="gmail-HOEnZb"><font color="#888888"><br>
-Eli<br>
<br>
-- <br>
Employee of Qualcomm Innovation Center, Inc.<br>
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project<br></font></span></blockquote><div> </div></div>-- <br><div class="gmail_signature">Saleem Abdulrasool<br>compnerd (at) compnerd (dot) org</div>
</div></div>