[PATCH] D73709: [PowerPC] Create a FixedStack object for CR save in linkage area.

[Sean Fertile via Phabricator via llvm-commits]

sfertile marked 2 inline comments as done.
sfertile added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCFrameLowering.cpp:1804
+  // function uses CR 2, 3, or 4. For 64-bit SVR4 we create a FixedStack
+  // object that aliases the CR-save slot in the linkage area. The actual save
+  // and restore of the condition register will be created as part of the
----------------
jsji wrote:
> `aliases`? looks confusing, as `IsAliased` below is always false.
Your right, the comment makes the code look like its passing the wrong value for the argument. I've reworded the comment.


================
Comment at: llvm/test/CodeGen/PowerPC/ppc64-alloca-crspill.ll:31
+
+; CHECK-NEXT: stack:
+; CHECK-NEXT:   - { id: 0, name: '<unnamed alloca>', type: variable-sized, offset: -8,
----------------
I'm not sure why the offset of the variably size object is -8, which overlaps with the fixed save slot for the frame pointer. It might be that the offset is not a valid field for variably sized objects? I'm going to investigate this now




================
Comment at: llvm/test/CodeGen/PowerPC/ppc64-alloca-crspill.ll:36
+; CHECK-NEXT:       debug-info-location: '' }
+; CHECK-NEXT:   - { id: 1, name: '', type: default, offset: -16, size: 8, alignment: 8,
+; CHECK-NEXT:       stack-id: default, callee-saved-register: '', callee-saved-restored: true,
----------------
I suspect this stack object is padding for keeping the stack pointer 16 byte aligned: 32 byte linkage area + 8 bytes spill for $x31 would have only been 8 bytes.


================
Comment at: llvm/test/CodeGen/PowerPC/ppc64-crsave.mir:10
+---
+name:            CRAllSave
+alignment:       16
----------------
jsji wrote:
> Can we add some test with alloca in callee stack?
Good idea. I've added it in a separate test since I though it was best to use IR input and stop after prologue/epilogiue insertion. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73709