Can you make this dependent on https://reviews.llvm.org/D76902and rebase/repost.

• rebase to pick up landed dependencies.
• Moved assertion that loop index was still valid to top of loop.
• added a test that exercise fatal error for args split between registers and stack.

Which I think mishandles negative values.

run clang-format, but otherwise for fixing the assertion it LGTM. FWIW I'm not sure the code we produce for the conversion handles negative numbers correctly. For the reduced IR I posted I get:

Thanks for the update, LGTM.

I would rather we don't rename any of the ViaCopy lists, or CSR_SRV464_TLS_PE_SaveList since they are likely dead. We can write IR that triggers them, but Clang will only generate said IR for Darwin IIUC and from the name of the calling convention it is limited to C++.
Also please clang format the changes.

LGTM.

Rebased

LGTM. Thanks for the updates Chris.

I suggest adding a test along the lines of:

Change looks good, however there is a couple things complicating this.

1. The SplitCSR code is dead with Darwins removal and we didn't realize it.
2. CSR_AIX64 and CSR_SVR464 are the same and should be one list, not 2 distinct lists.

1 comment, but otherwise LGTM.

Add a test that checks for a fatal error when a ByVal argument is split across registers and stack.

In D76902#1950131, @sfertile wrote:

In D76902#1945938, @ZarkoCA wrote:

A small suggestion would be to add a test to further show that the GPRs are "burned" properly with with floats in the byval handling. Eg. using a struct like `%struct.S31 = type <{ float, i32, i64, double, i32, double }>' and seeing the error that structs split regs and the stack are not handled.

This is a really good suggestion. Going to add this next.

In D76902#1945938, @ZarkoCA wrote:

A small suggestion would be to add a test to further show that the GPRs are "burned" properly with with floats in the byval handling. Eg. using a struct like `%struct.S31 = type <{ float, i32, i64, double, i32, double }>' and seeing the error that structs split regs and the stack are not handled.

• Fixed wording on a fatal error.
• Changed the args and captures on the lambda.
• Restructured the loop as suggested.
• Added comment about the dead-stores in the lit test.

One comment, but otherwise LGTM.

Thank you for the updates. One minor comment, but patch LGTM.

A couple minor suggestions. If you agree with them, feel free to land them as an NFC update to the test before commiting this patch.

LGTM.

In D76591#1939442, @MaskRay wrote:

In D76591#1939318, @jasonliu wrote:

@nemanjai
AIX system assembler does not seem to like this new form.

Assembler:
test.s: line 46: 1252-086 The target of the branch instruction
        must be a relocatable or external expression.

llvm-objdump -d output may not be assembled back. That said, I think most users like the PC relative immediate to be displayed as the target address. If AIX users don't agree, I can special case it.

Added a comment to describe why we must store the registers used to pas a ByVal to the stack.

• split the ByVal handling into memory and register parts to remove an 'else'.
• add back some missing info for fixed stack objects in the lit test.
• remove check lines related to fixed stack objects which are not semantically meaningful.

Run clang-format.

• Fixed assertion with zero sized byvals that new testing exposed.
• Addressed various review comments (renamed TrueSize, use alignTo, etc).

I'm going to implement the suggested changes to the testing (ie a zero-sized-byval and different byval types so we are loading more then just a char). I'll do so in a separate patch which makes the changes and shows the difference in the existing tests and then I'll rebase this patch onto that.

• rename local to match other uses in function.
• constify a handful of locals.

Landed check-prefix changes as an NFC patch and rebased ontop of that.

LGTM.

As a first step, I suggest we break the clang changes and the LLVM changes into 2 separate patches.

LGTM.

1 minor comment but otherwise LGTM.

I get why you named the test ppc32-setcc-expansion.ll but to a casual reader its definitely unclear what a setcc has to do with this code. I would suggest 'ppc32-i64-to-float-conv.ll', and have a comment explaining how the lowering for the conversion generates a setcc which caused a crash under certain conditions.

LGTM.

LGTM.

Reworded comment and added 64-bit RUN step to the test.

Please fix the formatting issues flagged by the pre-merge checks.

In D74225#1894003, @cebowleratibm wrote:

I think we need some changes to ensure frame objects are created with their proper size and not word size.

LGTM.

Fix the formatting that Lint called out, but otherwise LGTM for fixing building natively on AIX.

Fix formatting of CHECK lines in several of the lit tests.

Patch LGTM as far a formatting/naming/testing etc. C++ specifics is outside my wheelhouse though, so I can't confirm things like the tail padding rules are correct for AIX. Because of that I'm not comfortable being the one to accept the patch.

• Added a lambda for building the instruction which moves the condition register into a scratch register to avoid dupication.
• Added an IR --> assembly test that verifies we restore all 3 non-volatile cr-fields.
• Removed a couple stale comments, and reworded another comment.

LGTM.

Looks like there are calls to assembleToStream which didn't get updated to check that the return value is valid, breaking bots that treat warnings as errors: http://lab.llvm.org:8011/builders/clang-ppc64le-rhel/builds/1389

1. We should probably update the COMDAT section of the lang ref to mention XCOFF doens't support COMDATS.
2. Will we report an error somewhere in the backend if we encounter a COMDAT?