efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.
I guess... fine.
LGTM, assuming we have test coverage for all the different cases.
Repository:
rL LLVM
https://reviews.llvm.org/D39321
compnerd added a comment.
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
compnerd closed this revision.
compnerd added a comment.
SVN r316810
Repository:
rL LLVM
https://reviews.llvm.org/D39321
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
compnerd added a comment.
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.
Repository:
rL LLVM
https://reviews.llvm.org/D39321
efriedma added a comment.
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
compnerd added inline comments.
Comment at: lib/Basic/Targets/ARM.cpp:231
+ else if (Triple.isWatchABI())
+PtrDiffType = SignedLong;
+
I changed this to:
if (Triple.isOSDarwin() && !Triple.isWatchABI())
PtrDiffType = SignedInt;
Repository:
rL
compnerd updated this revision to Diff 120534.
compnerd added a comment.
Catch a couple of missed instances, add more context
Repository:
rL LLVM
https://reviews.llvm.org/D39321
Files:
lib/Basic/Targets/ARM.cpp
test/Preprocessor/init.c
Index: test/Preprocessor/init.c
efriedma added a comment.
I think you missed a couple other places which still set SizeType and
PtrDiffType?
Repository:
rL LLVM
https://reviews.llvm.org/D39321
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
fhahn added a comment.
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.
Repository:
rL LLVM
https://reviews.llvm.org/D39321
compnerd created this revision.
compnerd added a project: clang.
Herald added subscribers: kristof.beyls, javed.absar, aemerson.
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
10 matches
Mail list logo
