[PATCH] D95664: [AVR] Fix the eliminateCallFramePseudos method so that it always expands STDWSPQRr and STDSPQRr

[Matt Jacobson via Phabricator via llvm-commits]

mhjacobson added a comment.

In D95664#2660051 <https://reviews.llvm.org/D95664#2660051>, @arsenm wrote:

> I just realized I've been reading "frame pointer elimination" as "frame index elimination" but this change still doesn't make sense to me. Why do you want to treat these stores as frame instructions when they don't modify the stack pointer?

Stepping in to try to answer this since I'm interested in a fix here.

It's true that these stack-store pseudos do not modify the stack pointer.  However, their expansion depends on which register is used as the stack pointer (`Y` or `Z`), and `AVRFrameLowering::eliminateCallFramePseudoInstr()` is currently the place where that is known.

`PEI::replaceFrameIndices` calls `eliminateCallFramePseudoInstr()` only for instructions for which `isFrameInstr()` returns true.  When `AVRFrameLowering::eliminateCallFramePseudoInstr()` is called, `fixStackStores` is called in turn to expand any stack-store pseudos in the remainder of the basic block.  So making `isFrameInstr()` return true for stack-store pseudos causes `eliminateCallFramePseudoInstr()` to be called for them and `fixStackStores()` to expand them.  (At that point, the loop inside `fixStackStores()` becomes redundant, I think.)

Perhaps the work of expanding these pseudos should be done elsewhere?  `AVRExpandPseudoInsts` seems like the obvious place, but I'm not sure whether it has access to the requisite knowledge of which register contains the stack pointer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95664