A first pass over platform-specific properties of the C API/ABI on AIX for both 32-bit and 64-bit modes. This is a continuation of D18360 by Andrew Paprocki and further work by Wu Zhao.
Details
- Reviewers
apaprocki chandlerc hubert.reinterpretcast jasonliu xingxue sfertile - Commits
- rG7f7867b05ab7: Reland the rest of "Add AIX Target Info"
rC356208: Reland the rest of "Add AIX Target Info"
rL356208: Reland the rest of "Add AIX Target Info"
rGc4420b00f102: Reland part of "Add AIX Target Info"
rL356197: Reland part of "Add AIX Target Info"
rC356197: Reland part of "Add AIX Target Info"
rG4e192d0e1e72: Add AIX Target Info
rL356060: Add AIX Target Info
rC356060: Add AIX Target Info
Diff Detail
- Repository
- rC Clang
Event Timeline
clang/lib/Basic/Targets/OSTargets.h | ||
---|---|---|
626 ↗ | (On Diff #189580) | Comments should be full sentences (with periods). Please update throughout this patch. |
629 ↗ | (On Diff #189580) | @rsmith suggested that having a -fno-long-long option to disable support for long long would be preferable to having an -maix-no-long-long that just controls the AIX binary-compatibility aspects. Perhaps the FIXME should be updated to mention checking -fno-long-long. |
637 ↗ | (On Diff #189580) | Suggestion for the comment text: |
641 ↗ | (On Diff #189580) | D18360 sets _THREAD_SAFE to 1 when -pthread is specified. I believe that to be correct. I believe whether or not -pthread is taken to be the default on the platform is a separate consideration. |
651 ↗ | (On Diff #189580) | Can we have a test for this? |
clang/lib/Basic/Targets/OSTargets.h | ||
---|---|---|
626 ↗ | (On Diff #189580) | I do not think it is realistic to support an AIX target prior to 7.1, so should this unconditionally define up to and including _AIX71 similar to D18360 did (it was written earlier and defined up to _AIX61 unconditionally) and leave a FIXME to determine when to emit _AIX72 or any other later versions? |
clang/test/Preprocessor/init.c | ||
6420 ↗ | (On Diff #189580) | XL on AIX emits #define _LP64 1 in 64-bit mode and #define _ILP32 1 in 32-bit mode in the pre-defined macros. Is that important to capture? |
clang/lib/Basic/Targets/OSTargets.h | ||
---|---|---|
626 ↗ | (On Diff #189580) | I agree with your assessment re: older versions of AIX in terms of having a full solution for modern C++. Nevertheless, we can leave room open for hobby builds targeting older versions where the immediate cost is not high. Cross compiling to an older AIX target will probably be entirely possible for C. |
clang/test/Preprocessor/init.c | ||
6420 ↗ | (On Diff #189580) | I think so. The v16.1 XL compiler's xlclang also produces these. #define __LP64__ 1 #define _LP64 1 #define __ILP32__ 1 #define _ILP32 1 |
clang/test/Preprocessor/init.c | ||
---|---|---|
6420 ↗ | (On Diff #189580) | It seems GCC on AIX only defines the macros for 64-bit, and not the 32-bit versions. The system headers do not appear to depend on the 32-bit versions. It makes sense to start with the common intersection between the GCC and XL predefined macros first. We can add __LP64__ and _LP64 with this patch. I think we can leave more macros for later. |
clang/test/Preprocessor/init.c | ||
---|---|---|
6420 ↗ | (On Diff #189580) |
Agreed |
- Updated comments
- Added testing for UseZeroLengthBitfieldAlignment
- Defined _THREAD_SAFE when -pthread is enabled
- Check for definition of __LP64__ and _LP64 macros
clang/lib/Basic/Targets/OSTargets.h | ||
---|---|---|
641 ↗ | (On Diff #189580) | I've added a test in clang/test/Preprocessor/init.c to check if the macro is defined when -pthread is enabled. |
clang/test/CodeGen/arm-aapcs-zerolength-bitfield.c | ||
---|---|---|
5 ↗ | (On Diff #189890) | This file is pending a name change. |
clang/lib/Basic/Targets/OSTargets.h | ||
---|---|---|
640 ↗ | (On Diff #189890) | Line is longer than 80 characters. Please split it. |
clang/test/CodeGen/arm-aapcs-zerolength-bitfield.c | ||
1 ↗ | (On Diff #189890) | Given the requirement for arm-registered-target, is the file actually run whenever we intend it to be? Also, a note re: the existing test: This does not seem to be a CodeGen test; indeed, it is effective even with -fsyntax-only. It seems this should be moved to clang/test/Sema. However, not all of the cases are common anyway. We will later need to post changes that implement AIX's 4-byte storage units for bit-fields. I think we should leave this file alone for the purposes of this patch. |
clang/test/Preprocessor/init.c | ||
7027 ↗ | (On Diff #189890) | Add a check that _LP64 is not defined under the 32-bit mode. Similarly for __LP64__ and __64BIT__. Other targets also check for the presence or absence of _ILP32 and __ILP32__, so we probably should do the same. |
- Fixed comment.
- Checked that _LP64, __LP64__, and __64BIT__ aren't defined in 32 bit mode.
- Checked that _ILP32 and __ILP32__ aren't defined in 64 bit mode.
I'm seeing some build bot failures because of the newly added test case: max_align.c.
So I reverted the previous committed change.
Please fix the test case and let me know so that I can help you commit again.
test/Headers/max_align.c | ||
---|---|---|
1 | We may need to explicitly specify C11. It also seems that we should XFAIL Windows targets. |
test/Headers/max_align.c | ||
---|---|---|
1 | Does this need an explicit -triple=??? ? |
test/Headers/max_align.c | ||
---|---|---|
1 | I don't think this test is target-specific, I read the specifications (as opposed to the implementations) of max_align_t and __BIGGEST_ALIGNMENT__ as saying that _Alignof(max_align_t) should be the same as __BIGGEST_ALIGNMENT__. Granted, it seems max_align_t on Windows does not match __BIGGEST_ALIGNMENT__. So, a custom malloc implementation querying on one might not align sufficiently based on the other. |
We may need to explicitly specify C11. It also seems that we should XFAIL Windows targets.