[PATCH] D72454: [AIX] Enable frame pointer for AIX and add related test suite

[Zarko Todorovski via Phabricator via llvm-commits]

ZarkoCA added a comment.

I think this patch is really close, just needs some typo fixes.  I'm hesitant to approve since no one else has had a chance to look at it.



================
Comment at: llvm/lib/Target/PowerPC/PPCFrameLowering.cpp:63
 static unsigned computeFramePointerSaveOffset(const PPCSubtarget &STI) {
   // For the Darwin ABI:
   // We cannot use the TOC save slot (offset +20) in the PowerPC linkage area
----------------
Xiangling_L wrote:
> ZarkoCA wrote:
> > The comment talks about the Darwin ABI but it looks that part of the code is removed.  Should the comment be edited or am I misunderstanding? 
> It is because we have the same logic for SVR4, AIX and Darwin, so I skip the `isDarwinABI()` check. Maybe I can adjust the structure of the comments like:
> 
> ```
> Darwin comments...;
> SVR4 ABI/AIX ABI:...;
> 
> return STI.isPPC64() ? -8U : -4U;  
> 
> ```
> 
> Please let me know if you think it would help.
Yes, I think that's better thanks. 


================
Comment at: llvm/test/CodeGen/PowerPC/Frames-alloca.ll:37
+
+; Test verifies for AIX:
+; - Frame pointer register (r31) is saved/defined/restored.
----------------
I find the comments in the tests really helpful in understanding what is happening and what to expect in the assembly, thanks for including these. 


================
Comment at: llvm/test/CodeGen/PowerPC/aix32-frames-stack-floor.ll:1
+; RUN: llc -verify-machineinstrs < %s -mcpu=pwr4 -mattr=-altivec \
+; RUN: -mtriple=powerpc-ibm-aix-xcoff | FileCheck %s -check-prefix=PPC32-NOFP
----------------
Nit: can you combine this and aix43-frames-stack-floor.ll in one file? 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72454/new/

https://reviews.llvm.org/D72454