[PATCH] D140917: [AVR] Optimize away cpi instructions when possible

[Ben Shi via Phabricator via llvm-commits]

benshi001 added inline comments.


================
Comment at: llvm/lib/Target/AVR/AVRInstrInfo.cpp:202
+    CmpMask = ~0;
+    CmpValue = MI.getOperand(1).getImm();
+    return true;
----------------
It would be better to add a range check here.

1. The `MI.getOperand(1).getImm();` should be in range [0, 255], otherwise `llvm_unreachable()`;
2. The `CmpMask` should be 255, since it is `int64_t` type.


================
Comment at: llvm/lib/Target/AVR/AVRInstrInfo.cpp:207
+  // There are more compare instructions, but they're not implemented yet.
+  return false;
+}
----------------
Can the form be that

```
switch {
default:
  // TOTO: Implement more compare instructions.
  return false;

case AVR::CPIRdK:
  ..
}
```


================
Comment at: llvm/lib/Target/AVR/AVRInstrInfo.cpp:218
+    return false;
+  case AVR::ADCRdRr:
+  case AVR::ADDRdRr:
----------------
I think it would better to use a `std::array` than `switch`.


================
Comment at: llvm/lib/Target/AVR/AVRInstrInfo.cpp:233
+  case AVR::SUBRdRr:
+  case AVR::SUBIRdK:
+    return true;
----------------
Why `LSL`, `ROL` are not in this list ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140917