LGTM.

Thanks for the updates Zarko. I think we are almost there.

Hey Zarko. I really like how we only issue the warning when the struct is used, however I think this is still overly broad because of where the incompatibility lies between xlc and clang. I believe the layout of the structures will be the same for both compilers, and globals of this type will have the same alignment restrictions. A function like void baz(int *); will be compatible between the 2 compilers since the argument is just a pointer. The difference occurs when passing the structure by value on the stack, where xlc doesn't align the struct to the expected alignment, and clang/llvm does. Since the incompatibility is in the calling convention only when the struct is passed byval, that should be the only time we emit the diagnostic.

LGTM.

Thanks Steven. LGTM.

Thanks David.

LGTM Wael. Sorry for taking so long to get to it.

rebase patch and update the testing.

In D112086#3266644, @ldionne wrote:

Here is an example of a build that's failing: https://buildkite.com/llvm-project/libcxx-ci/builds/8104#3fcf3f17-bdfe-476b-85f7-314c82a378ff. D117512 is completely unrelated to filesystem, so I can't imagine it's an actual issue with D117512.

Rebase to trigger ci before committing, since I let this sit so long.

Fixed formatting.

LGTM.

• Fix if condition that broke the Debian buildbot.
• Remove 'Required: asserts' from the test as I've updated to use a pass that is run regardless of whether asserts are enabled or not.
• Remove some unintended formatting changes from the lit test.

LGTM!

Nice work Yousuf. Can you start by adding the context for the patch (for example if you used git diff or git show then adding -U9999 will give you 9999 lines of surrounding context in the diff), and running clang format on the patch?

Rebased to trigger CI.

For the 2 binary files being added, I think we should append xcoff- in the names.

Rebase on https://reviews.llvm.org/D111359 and remove XFAIL from now passing tests.

In D109090#3018978, @nemanjai wrote:

@adalava the symbol __stack_chk_guard resides in libc.so on FreeBSD (i.e. a different DSO from the compiled program).

LGTM, but since as you mentioned I originally wrote this I would fell better if @cebowleratibm reviewed and approved before committing as well.

LGTM.

I suggest we separate the clang change and testing into a standalone patch, and the llvm backend tests into a standalone patch which we can commit separately.

In D104639#2954342, @MaryamBen wrote:

I tried to make changes you told me to see if I could make it in time. Because tomorow is my last day at work, I don't have enough time to redo all the patchs especially the tests.

In D104639#2832518, @jhenderson wrote:

As a non-XCOFF developer, this mostly looks fine to me, but needs a test case.

Hi Maryam.

Add a couple more struct layouts to the testing to show cases diagnostic is not issued.

Fixed diagnostic to only emit when there is a bitfield member.

In D107506#2931095, @sfertile wrote:

Moved the diagnostic emission to Sema::ActOnTagFinishDefinition as suggested.

Moved the diagnostic emission to Sema::ActOnTagFinishDefinition as suggested.

LGTM, other then 1 small test update.

Good point Chris. The only difference in layout is related to bitfield members, so I have moved the warning to VerifyBitField as suggested.

Only emit diagnostic on bitfield members, which is the only difference in align(packed) behaviour XL and clang/xlclang.

• Fixed spelling mistake
• Check that VD is non-nulll earlier.
• added warning that the requested alignment is too small.
• renamed test

Thanks Steven, LGTM.

Fix formatting.

Don't update the unpacked field align based on IsPacked.

Fixed spelling and added a 'long long' zero width bitfield to the testing.

Spelling and formatting fixes, and added coverage for the XCOFF object writing.

clang-formatted

LGTM, thanks for finding and fixing this MaskRay.

In D102632#2787406, @jasonliu wrote:

LGTM on AIX perspective.
FYI, we will need two reviewers from libc++ review group or the maintainer of libc++ project to approve before we could land.

ping.

In D103696#2816619, @EGuesnet wrote:

Hello,
For you information, we have had trouble with arcanist configuration, and my account is used to push to review. Patch has been written by @MaryamBen, as confirmed by commit author.
Comments in push message are also made @MaryamBen.
Regards.