[PATCH] D33887: [mips][microMIPS] Extending size reduction pass with ADDIUSP and ADDIUR1SP

[Simon Dardis via Phabricator via llvm-commits]

sdardis accepted this revision.
sdardis added a comment.
This revision is now accepted and ready to land.

Can you re-upload this diff before committing? The paths are cut short, so they're missing test/ and lib/ respectively.

Otherwise, LGTM with some nits on the comments.



================
Comment at: Target/Mips/MicroMipsSizeReduction.cpp:35
   OT_OperandsAll, ///< Transfer all operands
+  OT_Operands02,  ///< Transfer opernads no 0 and no 2
+  OT_Operand2,    ///< Transfer just oppernad no 2
----------------
Nit: "Transfer opernads no 0 and no 2" -> "Transfer operands 0 and 2"


================
Comment at: Target/Mips/MicroMipsSizeReduction.cpp:36
+  OT_Operands02,  ///< Transfer opernads no 0 and no 2
+  OT_Operand2,    ///< Transfer just oppernad no 2
 };
----------------
Nit; oppernad  -> operand.


================
Comment at: Target/Mips/MicroMipsSizeReduction.cpp:153
+  // Attempts to reduce ADDIU into ADDIUSP instruction,
+  // returns true on success
+  static bool ReduceADDIUToADDIUSP(MachineInstr *MI,
----------------
Nit: full stop at the end of this sentence.


================
Comment at: Target/Mips/MicroMipsSizeReduction.cpp:158
+  // Attempts to reduce ADDIU into ADDIUR1SP instruction,
+  // returns true on success
+  static bool ReduceADDIUToADDIUR1SP(MachineInstr *MI,
----------------
Nit: full stop at the end of this sentence.


================
Comment at: Target/Mips/MicroMipsSizeReduction.cpp:259
 
+// Returns true if the value is ADDIUSP immediate
+static bool AddiuspImmValue(int64_t Value) {
----------------
Nit: "is ADDIUSP immediate" should be "is a valid immediate for ADDIUSP.".


https://reviews.llvm.org/D33887