[PATCH] D117426: [AVR] Only push and clear R1 in interrupts when necessary

[Ben Shi via Phabricator via llvm-commits]

benshi001 added inline comments.


================
Comment at: llvm/lib/Target/AVR/AVRISelDAGToDAG.cpp:460
+  // correctly zeroed in interrupts.
+  Ops.push_back(CurDAG->getRegister(AVR::R1, MVT::i8));
+
----------------
benshi001 wrote:
> I am OK with current form. Just one more suggestion, you can add a new case of `icall`, current tests only contains a direct call but no indirect call. And you can add the `icall` test either in this patch or a new patch.
I have debugged your code, and I find this line is unnecessary.

Since there is already such a line in AVRISelLowering.cpp / LowerCall():

```
  // Add argument registers to the end of the list so that they are known live
  // into the call.
  for (auto Reg : RegsToPass) {
    Ops.push_back(DAG.getRegister(Reg.first, Reg.second.getValueType()));
  }

  // The R1 register must be passed as an implicit register so that R1 is
  // correctly zeroed in interrupts.
  Ops.push_back(DAG.getRegister(AVR::R1, MVT::i8));
```

That line in LowerCall is always executed, no matter direct call or indirect call, so the implicit `R1` will be added twice for indirect calls.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117426