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

[Xiangling Liao via Phabricator via llvm-commits]

Xiangling_L marked 10 inline comments as done.
Xiangling_L added inline comments.


================
Comment at: llvm/test/CodeGen/PowerPC/Frames-dyn-alloca-with-func-call.ll:13
+
+; RUN: not llc -verify-machineinstrs < %s -mcpu=pwr4 -mattr=-altivec \
+; RUN: -mtriple=powerpc-ibm-aix-xcoff 2>&1 | FileCheck %s \
----------------
cebowleratibm wrote:
> I expect there is a dependency on D73209, which adds the stack arg work.  Perhaps it's cleaner if we make this patch dependent on that so that we can avoid churn.
I think if we want this patch have a dependency on stack arg work depends on which patch is likely to land first? If D73209 is close to land, then I think it makes sense to have this dependency?


================
Comment at: llvm/test/CodeGen/PowerPC/Frames-dyn-alloca-with-func-call.ll:69
+; PPC32-LINUX: stw 11, 24(31)
+; PPC32-LINUX: stwux
+; PPC32-LINUX: stwux
----------------
cebowleratibm wrote:
> are these stwux deliberately omitting the operands?
Yes, cuz the rest of `stwux` is doing almost the same thing to update the `SP` and store allocated area address.  And I focus more on `We actually have 9 instances of dynamic stack allocation, identified by the stdux used to update the back-chain link.`


================
Comment at: llvm/test/CodeGen/PowerPC/Frames-dyn-alloca.ll:35
+; PPC32-LINUX-LABEL: f1
+; PPC32-LINUX: stwu 1, -32(1)
+; PPC32-LINUX-NEXT: slwi 3, 3, 2
----------------
cebowleratibm wrote:
> I find it useful to insert whitespace to line up the assembly text.
Thanks for mentioning this, I will update it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72454