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

Tue Mar 30 15:45:53 PDT 2021

arsenm requested changes to this revision.
arsenm added a comment.
This revision now requires changes to proceed.

This seems like a confusion of what frame instructions means. What do these stores have to do with call frames? The stores should reference frame indexes representing fixed frame objects in the incoming arguments or local objects. The frame adjust instructions are for SP modifications in call sequences where instructions are directly referencing offsets off of the SP, which do not have corresponding frame indexes

================
Comment at: llvm/test/CodeGen/AVR/bug-2021-01-29-complex-frame-pointer-usage.ll:1-2
+; RUN: llc < %s -march=avr -mcpu=atmega328 -filetype=obj -o /dev/null --print-options 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR
+; RUN: llc < %s -march=avr -mcpu=atmega328 -filetype=obj | llvm-objdump -S - | FileCheck %s --check-prefix=CHECK-OBJDUMP
+
----------------
Disassembling here is a little bit weird. Why isn't directly emitting asm sufficent?

================
Comment at: llvm/test/CodeGen/AVR/bug-2021-01-29-complex-frame-pointer-usage.ll:14-17
+; The issue was that the ISel lowering pass would lower many stack slot stores to these
+; instructions, but the frame pointer elimination code (that is designed to rewrite these two
+; instructions to real instructions) was only designed to run for STDWSPQRr/STDSPQRr instructions
+; that appeared in the basic blocks that contained the FrameSetup/FrameDestroy instructions.
----------------
This sentence doesn't make sense to me. Frame index elimination is unrelated to frame setup / frame destroy. 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95664