In D124555#4651438, @hubert.reinterpretcast wrote:

In D124555#3571553, @hubert.reinterpretcast wrote:

From the AIX and z/OS perspective, the replacement of the two members with a member having two fields is not 100% binary compatible for all usage. At least if the basic_ios specialization is used as a non-virtual base class, any derived class members that could be allocated in the padding following the bool would now move. This could be preemptively avoided if the new class type is given a user-provided copy constructor or destructor and the new member made [[no_unique_address]]. I don't know if [[no_unique_address]] is usable is some form under C++98/03 though.

We may be able to use some form of packed too.

Hi @ldionne, It would be appreciated if you could take a look at this patch and provide guidance on how to proceed.

I am taking over this revision because @cebowleratibm is working on a different project.

In D129229#4646238, @EricWF wrote:

@xingxue I apologize on behalf of the project for dropping this? Are you still willing to land it. If so, we can have it reviewed and completed this week.

I've reverted this patch to unblock the internal AIX buildbot.

Confirm LGTM.

LGTM, provided the CI is clean.

Addressed comments:

• add instruction bl $+4 in the assembly code of the test case

Addressed comments.

• add code to handle the case where a function does not store the back chain (stores_bc is 0)
• swap the order of getting sigcontext from a signal trampoline frame, now trying stack + STKMINALIGN first and then stack + STKMIN
• remove the condition check when restoring the SP
• add the test case for the case where a function does not store the back chain

Addressed comments:

• removed mentioning of buying stack frame in a code comment
• added if (!lastStack) and under the condition, set returnAddress to NULL
• removed condition TBTable->tb.saves_lr || !lastStack when restoring SP
• added code to initialize the LR value in context to 0

Addressed a comment:

• fix the comment - not all branches change LR.

GCC uses __register_frame_info and friends on platforms where the eh_frame_hdr/eh_frame sections are not supported such as AIX. But we can turn the macro on if GCC compat is needed on those OSs.

Thanks for working on this feature! LGTM once CI is fixed, which is a clang-format issue.

This patch is specific to the support for AIX legacy xlclang++ compiler generated code. It has been reviewed and approved by AIX compiler/runtime experts. If there are no objections, I will commit it tomorrow.

Diff with context.

Addressed comment:

• Call abort() instead of std::abort().

Changed function name from skipNonCxxEHAwareFrames to skip_non_cxx_eh_aware_frames to be consistent with function naming in snake_case in this file.

Addressed comments:

• changed a comment as suggested to be more descriptive
• fixed the text of a trace print

In D143010#4098038, @hubert.reinterpretcast wrote:

I made changes to the patch description (which I expect we will be using for the commit message).

Addressed comments:

• removed an unnecessary cast
• call std::abort() instead of std::terminate() when unwinding reaches the end of stack frames
• update callerStack instead of using another variable callerStack2
• added missing reference to t2.cpp source in the 32-bit test case

Addressed comments.

• added test scenarios for option fopenmp in clang/test/Driver/aix-ld.c

LGTM; thanks!

In D139368#3994092, @MaskRay wrote:

In D139368#3978330, @daltenty wrote:

This broke the AIX CI: https://buildkite.com/llvm-project/libcxx-ci/builds/15656#0184eac6-521b-4d1e-8502-c226673e0ece. Unfortunately it seems the .irp directive isn't supported by the AIX assembler.

What chunks of code do AIX use? All __powerpc__ and __powerpc64__ with all variants including __VSX__?

Addressed comments:

• use instruction vspltisb to initialize RS63, RS62
• set RS63, RS62 to 16 bytes each with value 0x01 and 0x02 respectively
• compare RS63/RS62 against expected values using instruction vcmpequb

In D138667#3949860, @ldionne wrote:

Getting libc++ out of the way. If it looks good to AIX folks, feel free to ship it.

Thanks!

The compiler generated prologue/epilogue uses instructions stvx/lvx to save/restore vector registers onto/from the stack. On the other hand, libunwind currently uses stxvd2x/lxvd2x to save/restore VSX vector to/from unwinder's context. Since stxvd2x/lxvd2x stores/loads VSX vector as 2 double words into/from memory, these instructions essentially change the byte orders of the contents if the machine is in little-endian mode. As a result, VSX vectors saved by prologues are not correctly represented in the unwinder context. PowerPC ISA 3.0 (Power9) has added instructions stxv/lxv that stores/loads VSX vectors and matches the behavior of stvx/lvx. Unfortunately, since supporting of older PowerPCs such as Power8 and earlier are still needed, we cannot replace stxvd2x/lxvd2x with stxv/lxv yet. Checking if the build system has the GLIBC facility for detecting the CPU architecture, and detecting at run time and going a different path add a lot of complicity to the code base. IMHO, we can simply add instruction xxswapd to before stxvd2x and after lxvd2x for little-endian like in your patch as a workaround for now. xxswapd is not an expensive instruction so it is unlikely to affect the performance in a significant way, noting that saving and restoring context are only executed when raising an exception and jumping to the landing pad. It will be good though to have a comment stating that we should replace them with stxv/lxv once only Power9 and higher are supported for little-endian PowerPC (RHEL 9.0 already supports minimum Power9). Thanks very much for working on this issue!

@ldionne, can you please take a look for #libc_abi? Thanks!

Addressed comments:

• rename AIXLongDoubleBuiltins to AIXLongDouble64Builtins
• test IR output instead of assembly output
• fix a typo in a comment
• mention frexpl(), ldexpl(), and modfl() are for 128-bit IBM long double, i.e. __ibm128

Thanks, @ldionne!

This patch is AIX specific and it has been thoroughly reviewed and approved by AIX compiler experts @hubert.reinterpretcast, @cebowleratibm (Thank you!). Since we need to have the patch in the coming service pack, I've committed it in rGa499051f10a2d0150b60c14493558476039f701a. Commit rGfb391e45e9cc35429f077413ca660f4f5f5212c1 modified a comment.

In D136770#3886558, @MaskRay wrote:

[libc++abi][CMake] Handle build type RelWithDebInfo configuration similarly to Release

Mention MINSIZEREL as well.

From reading https://github.com/Kitware/CMake/blob/master/Modules/Compiler/GNU.cmake#L58 , I think this makes sense.

Refreshed the patch with context.

Changes in the update:

• addressed comment: rename dummy() to force_a_stackframe();
• added attribute optnone to force_a_stackframe();
• use assembly for IBM legacy compiler xlclang++ generated test cases instead of binary object files.

Updated diff to include binary files.

Addressed comments:

• always define before_epoch_time & friends to reduce the scope of the workaround.

Addressed comment.

• _Unwind_FindEnclosingFunction() returns NULL if argument pc is NULL;
• dropped else for returning NULL as suggested.

Hi @ldionne, What do you think about this? The affected test case works with GCC.

Gentle ping.

I'm not opposed to the change, but I wonder if we should consider placing this in an #if defined(_AIX) guard. This would avoid touching errno on other platforms, which would likely > be a TLS access. It would also make it more obvious that this is a workaround for AIX. At the very least, I think that we should add some comments to explain the issue so that it is > obvious at the site rather than having to go through the change log.