[PATCH] D114611: [AVR] Expand STDWSPQRr & STDSPQRr, approach #2

Sun Dec 12 09:12:19 PST 2021

Patryk27 added inline comments.

================
Comment at: llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp:1221
+                              ? AVR::R29R28
+                              : AVR::R31R30);
+
----------------
couchand wrote:
> Perhaps I'm missing something, but where is Z being initialized?  I'm happy with the use of Y (I'm fine trusting the comment in the old code that Y is guaranteed to hold a copy of SP, though maybe I shouldn't just trust that).
> 
> The old code initialized Z immediately prior to converting the pseudo stores into real stores.  I can't see the same guarantee being made here.  Do we know that Z still holds the adjusted frame pointer value?  At first glance it seems like intervening instructions could easily clobber it.
That's a great question, and the answer is: I'm not sure 😅

On a parallel matter, I wonder why we're using the `Z` in the first place - couldn't we always refer to R29R28? Seems like that's what the other backends (e.g. RISC-V) do.

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

https://reviews.llvm.org/D114611