This patch is to fix lit test case failure relate to alignment, on z/OS, maximum alignment value for 64 bit mode is 16 and also fixed clang/test/Layout/itanium-union-bitfield.cpp, attribute ((aligned(4))) is needed for bit-field member in Union for z/OS because single bit-field has one byte alignment, this will make sure size and alignment will be correct value on z/OS.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
@NancyWang2222, the patch description doesn't seem to say much about the bit-field case. Can you expand it?
clang/test/Layout/itanium-union-bitfield.cpp | ||
---|---|---|
4–5 | This comment is supposed to be true for both of the bit-field declarations, no? | |
7 | I think the macro should be in all caps. | |
20 | I can't help but notice that this does not have the attribute applied. | |
35 | This output does not match the behaviour of the xlclang++ compiler I tried: union B { char f1: 35; B(); }; B::B() {} extern "C" int printf(const char *, ...); int main(void) { B b[1]; printf("%lld\n", (long long)((char *)(b + 1) - (char *)b)); } Gives me: 5 as the size of the union. |
Hi Nancy, can you set this patch as a child revision of https://reviews.llvm.org/D98864 which sets the maximum alignment.
clang/test/Layout/itanium-union-bitfield.cpp | ||
---|---|---|
7 | I will fix it. Thanks | |
20 | it doesnt fail on union B. dump shows:
which is what test case expects | |
35 | You are right. xlclang++ has different output from clang. will need to look into it. |
clang/test/Layout/itanium-union-bitfield.cpp | ||
---|---|---|
20 | Just pointing out that the comment above and the xlclang++ behaviour below both seem to imply that adding the alignment attribute here should be done. If adding it is currently harmless, then I think it makes sense to add it. |
added. thanks
clang/test/Layout/itanium-union-bitfield.cpp | ||
---|---|---|
20 | sure. I will add attributes for union B as well. Thanks for pointing it out. |
This comment is supposed to be true for both of the bit-field declarations, no?