[AIX] Implement LR prolog/epilog save/restore

[cebowleratibm]

Summary

This patch fixes the offsets of fields in the stack frame linkage save area for AIX.

[jsji]

FYI. You can Edit Related Revision to set the parent revision to show dependency.

[sfertile]

I'll have to dig into the AIX ABI docs before I can review this more thoroughly but it looks good. I suggest we split out renaming of the hardcoded '8' to --> getCRSaveOffset() as a separate NFC patch and that we can land immediately.

llvm/lib/Target/PowerPC/PPCFrameLowering.cpp

796 ↗

(On Diff #208696)

minor nit: Can we just say Unsupported PPC ABI?

804 ↗

(On Diff #208696)

FWIW according to the original commit that guards this for ELF it shouldn't be needed for AIX either:

commit 38d945872097549606ea62b64ded743afdab0648
Author: Bill Schmidt <[e-mail]>
Date:   Wed Oct 10 20:54:15 2012 +0000

    The PowerPC VRSAVE register has been somewhat of an odd beast since
    the Altivec extensions were introduced.  Its use is optional, and
    allows the compiler to communicate to the operating system which
    vector registers should be saved and restored during a context switch.
    In practice, this information is ignored by the various operating
    systems using the SVR4 ABI; the kernel saves and restores the entire
    register state.  Setting the VRSAVE register is no longer performed by
    the AIX XL compilers, the IBM i compilers, or by GCC on Power Linux
    systems.  It seems best to avoid this logic within LLVM as well.
    
    This patch avoids generating code to update and restore VRSAVE for the
    PowerPC SVR4 ABIs (32- and 64-bit).  The code remains in place for the
    Darwin ABI.
    
    llvm-svn: 165656

[cebowleratibm]

Rebased changes after D63738 and

commit 9bd22fec0d7bee6fa32479ba090b9c89656c0a3c
Date: Fri Jul 26 14:02:17 2019 +0000

[PowerPC] Add getCRSaveOffset to improve readability. [NFC]

Other NFC change to update assertion text.

[hubert.reinterpretcast]

llvm/lib/Target/PowerPC/PPCFrameLowering.cpp

58 ↗

(On Diff #212344)

The patch does not add testing for this.

923 ↗

(On Diff #212344)

Minor nit: Let's treat report_fatal_error messages as consisting of full sentences. This one should end in a period.

804 ↗

(On Diff #208696)

It is also documented by the Assembler Language Reference for AIX version 7.2 that VRSAVE is not to be used or modified by AIX ABI-compliant programs.

llvm/test/CodeGen/PowerPC/aix-lr.ll

13 ↗

(On Diff #212344)

Can we check for the stack pointer adjustment in the prologue? As it is, I am under the impression that the saved link register would be overwritten by the call.

22 ↗

(On Diff #212344)

Same comment.

[hubert.reinterpretcast]

llvm/lib/Target/PowerPC/PPCFrameLowering.cpp

62 ↗

(On Diff #212344)

This patch does not add testing for this.

83 ↗

(On Diff #212344)

This patch does not add testing for this.

87 ↗

(On Diff #212344)

This looks like a copy/paste error (copied from line 71).

90 ↗

(On Diff #212344)

This logic does not seem to be common for 32-bit SVR4 and 32-bit AIX. See PPCFrameLowering::determineCalleeSaves:

// Reserve stack space for the PIC Base register (R30).
// Only used in SVR4 32-bit.

[sfertile]

llvm/lib/Target/PowerPC/PPCFrameLowering.cpp

62 ↗

(On Diff #212344)

Chris and I had briefly discussed this offline and I asked that we add asserts (or errors) if we need a frame-pointer or base-pointer on AIX. We can then add the testing and remove the assert/errors in a subsequent patch

[cebowleratibm]

llvm/lib/Target/PowerPC/PPCFrameLowering.cpp

58 ↗

(On Diff #212344)

Because the TOC save/restore is typically handled in the glue code / nop that follows the call, I don't have a reliable way to exercise this functionality. I believe this offset comes into play for indirect calls, however, we haven't implemented that for AIX yet.

I'll add a report_fatal_error on the code path but leave the new logic in place.

87 ↗

(On Diff #212344)

I'll defer this to the FP/BP follow on work.

90 ↗

(On Diff #212344)

to be addressed in FP/BP follow on work.

796 ↗

(On Diff #208696)

Updated the assertion text.

804 ↗

(On Diff #208696)

Noted and we should simplify this in a separate patch when we remove Darwin support.

llvm/test/CodeGen/PowerPC/aix-lr.ll

13 ↗

(On Diff #212344)

"stwu 1, -64(1)" is the SP adjustment in the prolog. Why do you think the saved link register would be overwritten by the call? This code saves the LR at offset 8 from SP, then saves the SP at offset 0 from SP and decrements SP by 64.

[cebowleratibm]

Update the TOC, FP and BP offset accessors to assert for AIX. The assertions will be removed with future work that accompanied by appropriate test coverage.

[hubert.reinterpretcast]

LGTM.

llvm/test/CodeGen/PowerPC/aix-lr.ll

13 ↗

(On Diff #212344)

I missed that this was using the update form of the instruction. Sorry for the noise.

[hubert.reinterpretcast]

My attempt to commit this only committed the test. I'll try again once the bots clear.

llvm/trunk/test/CodeGen/PowerPC/aix-lr.ll

+; RUN: llc -verify-machineinstrs -mtriple powerpc-ibm-aix-xcoff < %s | \
+; RUN: FileCheck --check-prefix=32BIT %s
+
+; RUN: llc -verify-machineinstrs -mtriple powerpc64-ibm-aix-xcoff < %s | \
+; RUN: FileCheck --check-prefix=64BIT %s
+
+define void @bar() {
+entry:
+
+; 32BIT: mflr 0
+; 32BIT: stw 0, 8(1)
+; 32BIT: stwu 1, -64(1)
+; 32BIT: bl .foo
+; 32BIT: nop
+; 32BIT: addi 1, 1, 64
+; 32BIT: lwz 0, 8(1)
+; 32BIT: mtlr 0
+
+; 64BIT: mflr 0
+; 64BIT: std 0, 16(1)
+; 64BIT: stdu 1, -112(1)
+; 64BIT: bl .foo
+; 64BIT: nop
+; 64BIT: addi 1, 1, 112
+; 64BIT: ld 0, 16(1)
+; 64BIT: mtlr 0
+
+  call void bitcast (void (...)* @foo to void ()*)()
+  ret void
+}
+
+declare void @foo(...)