[PATCH] D15144: [mips[microMIPS]] Adding code size reduction pass for MicroMIPS

[Simon Dardis via Phabricator via llvm-commits]

sdardis added inline comments.


================
Comment at: lib/Target/Mips/MicroMipsSizeReduction.cpp:158
+// ReduceType as a sub-criterion (when wide opcodes are the same)
+llvm::SmallVector<ReduceEntry, 32> MicroMipsSizeReduce::ReduceTable = {
+
----------------
Can this be reduced in size to 16 or 8 entries?


================
Comment at: lib/Target/Mips/MicroMipsSizeReduction.cpp:263-285
+bool MicroMipsSizeReduce::ReduceLWtoLWSP(MachineInstr *MI,
+                                         const ReduceEntry &Entry) {
+
+  if (!ImmInRange(MI, Entry))
+    return false;
+
+  if (!IsSP(MI->getOperand(1)))
----------------
These two functions can be folded together as ReduceXWtoXWSP with a comment stating it covers lwsp, swsp.


================
Comment at: lib/Target/Mips/MicroMipsSizeReduction.cpp:310-312
+    if (MI->isBundle() || MI->isTransient()) {
+      continue;
+    }
----------------
Style: drop the '{' '} when it is a single line.


================
Comment at: lib/Target/Mips/MicroMipsSizeReduction.cpp:315-317
+    if (ReduceMI(MII)) {
+      Modified = true;
+    }
----------------
Modifed |= Reduced(MI); is clearer.


================
Comment at: lib/Target/Mips/MicroMipsSizeReduction.cpp:327
+  MI->setDesc(MipsII->get(Entry.NarrowOpc()));
+  DEBUG(errs() << "Converted into 16-bit: " << *MI);
+  ++NumReduced;
----------------
This should be dbgs() << ...


================
Comment at: test/CodeGen/Mips/micromips-sizereduction/micromips-lwsp-swsp.ll:6
+entry:
+; CHECK-LABEL: function1
+; CHECK: SWSP_MM
----------------
Colon after the function name so that it matches properly.


https://reviews.llvm.org/D15144