[PATCH] D95664: [AVR] Fix the eliminateCallFramePseudos method so that it always expands STDWSPQRr and STDSPQRr
Matt Jacobson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 21 11:05:47 PDT 2021
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
More information about the llvm-commits
mailing list