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

[Patryk Wychowaniec via Phabricator via cfe-commits]

Patryk27 marked 6 inline comments as done.
Patryk27 added a comment.

Thanks for the review!

For the time being, I have extracted the first set of changes into:
https://reviews.llvm.org/D122533



================
Comment at: llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp:1234
+
+  MachineOperand &Dst = MI.getOperand(0);
+  Register DstReg = Dst.getReg();
----------------
benshi001 wrote:
> Why we need an extra `Dst` local variable here? I did not find there is any more use besides `.getReg()` .
Right, I think it must have been an oversight on my side; I have fixed it in the follow-up merge request (https://reviews.llvm.org/D122533).


================
Comment at: llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp:1246
+  // few operations
+  if (Imm >= 63) {
+    if (!DstIsKill) {
----------------
benshi001 wrote:
> I suggest we make another patch for merge `AVRRelaxMem::relax<AVR::STDWPtrQRr>` into `expand<AVR::STDWPtrQRr>`, for the case `Imm >= 63`. And we select that merge patch as baseline / parent of current patch.
Sure! https://reviews.llvm.org/D122533


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114611