[PATCH] D28121: [AVR] Optimize 16-bit ORs with '0'

[Dylan McKay via Phabricator via llvm-commits]

dylanmckay added a comment.

Looking really nice! Added a few nits



================
Comment at: lib/Target/AVR/AVRExpandPseudoInsts.cpp:77
   bool expandLogicImm(unsigned Op, Block &MBB, BlockIt MBBI);
+  bool expandLogicImmNeeded(unsigned Op, unsigned ImmVal);
 
----------------
As this method doesn't actually do any expansion and only returns a boolean, you should rename it to something like `isLogicImmOpRedundant`.

Also, you should add a `const` qualifier to the method.


================
Comment at: lib/Target/AVR/AVRExpandPseudoInsts.cpp:225
   TRI->splitReg(DstReg, DstLoReg, DstHiReg);
+  bool Expand;
 
----------------
This temporary variable is unnecessary, plus it would be clearer if it looked `if (expandLogicImmNeeded(..) { }`


https://reviews.llvm.org/D28121