efriedma added inline comments.
================
Comment at: llvm/lib/Target/ARM/ARMFrameLowering.cpp:1844
+ for (const auto &MI : MBB)
+ if (MI.getOpcode() == ARM::tSTRspi || MI.getOpcode() == ARM::tSTRi)
+ for (const auto &Op : MI.operands())
----------------
How do you end up with tSTRi with a frameindex operand? SelectionDAG won't
generate that, I think.
================
Comment at: llvm/lib/Target/ARM/ARMFrameLowering.cpp:2110
+ if ((requiresAAPCSFrameRecord(MF) ||
+ MF.getTarget().Options.DisableFramePointerElim(MF)) &&
+ !LRSpilled) {
----------------
Should requiresAAPCSFrameRecord() be orthogonal to DisableFramePointerElim()?
I mean, we have a complete set of flags controlling frame pointer elimination;
the only part that's "new" here is that the frame pointer is stored in r11,
instead of r7.
================
Comment at: llvm/lib/Target/ARM/ARMFrameLowering.cpp:2317
+ if ((BigFrameOffsets || canSpillOnFrameIndexAccess(MF, *this)) &&
+ !ExtraCSSpill) {
// If any non-reserved CS register isn't spilled, just spill one or two
----------------
Rearrange this to check ExtraCSSpill first, so we don't run
canSpillOnFrameIndexAccess() if it isn't necessary?
================
Comment at: llvm/test/CodeGen/Thumb/frame-access.ll:73
+; CHECK-AAPCS: mov r0, r11
+; CHECK-AAPCS: str r1, [r0, #8]
+; CHECK-AAPCS: mov r0, r11
----------------
Can we use sp-relative accesses here? If we're not doing dynamic allocations,
it should be a fixed offset.
================
Comment at: llvm/test/CodeGen/Thumb/frame-access.ll:96
+; CHECK-ATPCS: add r7, sp, #8
+; CHECK-AAPCS: add r11, sp, #0
; Align stack
----------------
I don't think "add r11, sp, #0" corresponds to a legal Thumb1 instruction?
(Does -verify-machine-instrs catch this?)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125094/new/
https://reviews.llvm.org/D125094
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
