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

[Sean Fertile via Phabricator via llvm-commits]

sfertile added inline comments.


================
Comment at: llvm/test/CodeGen/PowerPC/Frames-dyn-alloca-with-func-call.ll:29
+
+; Test verifies:
+; - Frame pointer register (r31) is saved/defined/restored.
----------------
I have a couple questions about this test.

> We actually have 9 instances of dynamic stack allocation, identified by the stdux used to update the back-chain link.
Why 9 allocas? What is that exercising that say 2 or maybe 3 allocas wouldn't? The way this is worded it might imply that 9 dynamic allocations in the IR *must* produce 9 `stdux` instructions. Is that intended?

> Allocated area is 4 bytes aligned in the case of int.
Where do we verify the stack adjustments are 4 byte aligned?  Would it be more appropriate to say 'is at least 4 byte aligned' instead?


================
Comment at: llvm/test/CodeGen/PowerPC/Frames-large.ll:29
+
+; Test verifies:
+; - Frame pointer register (r31) is saved/defined/restored when frame pointer
----------------
Does this test warrant such a detailed description? To me it seems obvious what we are testing: updating the stack frame when the adjustment is too large to be encoded in an immediate.


================
Comment at: llvm/test/CodeGen/PowerPC/Frames-small.ll:30
+; Test verifies:
+; - Frame pointer register (r31) is saved/defined/restored when frame pointer
+;   elimination is disabled.
----------------
Ditto on the description being much too verbose for the test.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-frames-stack-floor.ll:13
+
+; Test verifies:
+; - Frame pointer register (r31) is saved/defined/restored when frame pointer
----------------
Ditt on on the description being overly verbose. This test is simple enough I think its completely self describing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72454