Page MenuHomePhabricator

stdint.h should use the target's types, cleanup the targets
AcceptedPublic

Authored by majnemer on Oct 24 2013, 5:29 PM.

Details

Summary

We would not choose the proper type for [u]intptr_t if we were
targeting -ffreestanding mode because we were substituting an equally
sized type, not the exact type.

An examination of Basic/Targets.cpp showed an irregular smattering of
target choices where we would get things wrong. This code has been
reoriented around Operating Systems since they are largely uniform
outside of ILP32-LLP64 differences.

This has found a few notable bugs:

  • MIPS and ARM would have their size_t as int and their ptrdiff_t as long
  • aarch64 on Linux had it's 64-bit type set as 'long long' instead of as long

Diff Detail

Event Timeline

majnemer updated this revision to Unknown Object (????).Oct 24 2013, 5:45 PM

Forgot DragonFlyBSD.

majnemer updated this revision to Unknown Object (????).Oct 24 2013, 7:12 PM
  • Remove unused targets, fixup Minix and kFreeBSD

I've looked over the bits I know and have access to, and it looks mostly fine to me. I don't know anything about the BSDs or anything though.

lib/Sema/SemaType.cpp
4792–4793

I think these two should probably be removed now that vectors are Long/ULong.

majnemer added inline comments.Oct 25 2013, 3:12 AM
lib/Sema/SemaType.cpp
4792–4793

I tried that and got bit by 32-bit ARM, they use long long for int64_t.

I think these two should probably be removed now that vectors are Long/ULong.

I tried that and got bit by 32-bit ARM, they use long long for int64_t.

Ah, of course! I forgot we shared that bit of code. Sorry about that.

Tim.

As to MIPS target the patch looks good to me.

The ARM changes look good to me.

There are is no test of aarch64 to show that "aarch64 on Linux had it's 64-bit type set as 'long long' instead of as long" has changed.

majnemer updated this revision to Unknown Object (????).Oct 27 2013, 10:29 PM
  • Remove unused targets, fixup Minix and kFreeBSD
  • Do a better job of matching GCC for OS-free targets
  • Fix ARM for NetBSD
majnemer updated this revision to Unknown Object (????).Oct 27 2013, 10:47 PM
  • Add a test checking that int64_t is a signed long instead of a signed long long
scott-0 added inline comments.Oct 28 2013, 6:32 AM
test/Preprocessor/init.c
253

Is it a problem that __INTPTR_TYPE__ and __INTPTR_WIDTH__ have been lost for some targets? (ARM, MIPS*, PPC*, ...)

majnemer added inline comments.Oct 28 2013, 11:02 AM
test/Preprocessor/init.c
253

INTPTR_TYPE and INTPTR_WIDTH are only defined by operating system targets in GCC's config files, I follow that convention.

Sorry, I didn't realize I was a reviewer and hence need to respond. I guess I'm here because I tinkered with the PS3 stuff, and being with Sony, perhaps because of the deletion of the PSP and SPU stuff. I don't even know if it was done by Sony folks (the log says evocallaghan for the PSP part), or why it needs to be deleted, but I have no problem with it. Let me ask around here a bit first to see if anyone here knows about it.

-John

Sony hasn't used the PSP or SPU targets that were built by others. However, the PS3PPU target is in active use.

Otherwise, I can't meaningfully review this patch because the are just too many changes combined in this patch.

@alexr I put all of the changes in one patch because splitting them into smaller patches risks inconsistency in the tree. Do you have a suggestion on how you would like this patch to look?

t.p.northover accepted this revision.Apr 3 2014, 4:46 AM

What is the state of this revision? Maybe abandon or close it.

jtsoftware resigned from this revision.May 5 2014, 6:51 PM
jtsoftware removed a reviewer: jtsoftware.

Sorry, I don't feel qualified to review it.

joerg edited edge metadata.Jul 29 2014, 5:42 AM

I've some errors in this area recently, so does this change still apply?

atanasyan resigned from this revision.Feb 2 2016, 10:25 PM
atanasyan removed a reviewer: atanasyan.
espindola edited reviewers, added: espindola; removed: rafael.Mar 14 2018, 4:58 PM